Skip to content
Permalink
Browse files
OAK-9739 : Avoid duplicate tree resolution in MembershipProvider
  • Loading branch information
anchela committed Mar 31, 2022
1 parent e38d861 commit 5a1a902e7a89fc44cb9e2f59b0c6939efa9c16e4
Showing 10 changed files with 138 additions and 122 deletions.
@@ -24,7 +24,6 @@
import javax.jcr.query.Query;

import com.google.common.base.Function;
import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import org.apache.jackrabbit.JcrConstants;
@@ -148,6 +147,12 @@ public Tree getTree(String identifier) {
}
}

@Nullable
public Tree getTree(@NotNull PropertyValue referenceValue) {
checkType(referenceValue.getType());
return resolveUUIDToTree(referenceValue);
}

/**
* The path of the tree identified by the specified {@code identifier} or {@code null}.
*
@@ -173,12 +178,8 @@ public String getPath(String identifier) {
*/
@Nullable
public String getPath(PropertyState referenceValue) {
int type = referenceValue.getType().tag();
if (type == PropertyType.REFERENCE || type == PropertyType.WEAKREFERENCE) {
return resolveUUID(PropertyValues.create(referenceValue));
} else {
throw new IllegalArgumentException("Invalid value type");
}
checkType(referenceValue.getType());
return resolveUUID(PropertyValues.create(referenceValue));
}

/**
@@ -191,12 +192,8 @@ public String getPath(PropertyState referenceValue) {
*/
@Nullable
public String getPath(PropertyValue referenceValue) {
int type = referenceValue.getType().tag();
if (type == PropertyType.REFERENCE || type == PropertyType.WEAKREFERENCE) {
return resolveUUID(referenceValue);
} else {
throw new IllegalArgumentException("Invalid value type");
}
checkType(referenceValue.getType());
return resolveUUID(referenceValue);
}

/**
@@ -300,11 +297,11 @@ public String apply(PropertyState pState) {
* @param propertyName The name of the reference properties.
* @param ntName The node type name to be used for the query.
* @param weak if {@code true} only weak references are returned. Otherwise on hard references are returned.
* @return A set of oak paths of those reference properties referring to the
* @return A set of oak trees containing reference properties referring to the
* specified {@code tree} and matching the constraints.
*/
@NotNull
public Iterable<String> getReferences(@NotNull Tree tree, @NotNull final String propertyName,
public Iterable<Tree> getReferences(@NotNull Tree tree, @NotNull final String propertyName,
@NotNull String ntName, boolean weak) {
if (!effectiveNodeTypeProvider.isNodeType(tree, JcrConstants.MIX_REFERENCEABLE)) {
return Collections.emptySet(); // shortcut
@@ -321,18 +318,8 @@ public Iterable<String> getReferences(@NotNull Tree tree, @NotNull final String
QueryEngine.INTERNAL_SQL2_QUERY,
Query.JCR_SQL2, bindings, NO_MAPPINGS);

Iterable<String> resultPaths = Iterables.transform(result.getRows(), new Function<ResultRow, String>() {
@Override
public String apply(ResultRow row) {
return PathUtils.concat(row.getPath(), propertyName);
}
});
return Iterables.filter(resultPaths, new Predicate<String>() {
@Override
public boolean apply(String path) {
return !path.startsWith(VersionConstants.VERSION_STORE_PATH);
}
}
Iterable<Tree> resultTrees = Iterables.transform(result.getRows(), (Function<ResultRow, Tree>) row -> row.getTree(null));
return Iterables.filter(resultTrees, tree1 -> !tree1.getPath().startsWith(VersionConstants.VERSION_STORE_PATH)
);
} catch (ParseException e) {
log.error("query failed", e);
@@ -382,4 +369,11 @@ private Tree resolveUUIDToTree(@NotNull PropertyValue uuid) {
private static PropertyValue createPropertyValue(@NotNull String uuid) {
return PropertyValues.create(StringPropertyState.stringProperty("", uuid));
}

private static void checkType(@NotNull Type propertyType) {
int type = propertyType.tag();
if (!(type == PropertyType.REFERENCE || type == PropertyType.WEAKREFERENCE)) {
throw new IllegalArgumentException("Invalid value type");
}
}
}
@@ -283,13 +283,13 @@ private Iterator<Group> getMembership(boolean includeInherited) throws Repositor
Iterator<Group> dynamicGroups = dmp.getMembership(this, includeInherited);

MembershipProvider mMgr = getMembershipProvider();
Iterator<String> oakPaths = mMgr.getMembership(getTree(), includeInherited);
Iterator<Tree> trees = mMgr.getMembership(getTree(), includeInherited);

if (!oakPaths.hasNext()) {
if (!trees.hasNext()) {
return dynamicGroups;
}

AuthorizableIterator groups = AuthorizableIterator.create(oakPaths, userManager, AuthorizableType.GROUP);
AuthorizableIterator groups = AuthorizableIterator.create(trees, userManager, AuthorizableType.GROUP);
AuthorizableIterator allGroups = AuthorizableIterator.create(true, dynamicGroups, groups);
return new RangeIteratorAdapter(allGroups);
}
@@ -19,6 +19,7 @@
import com.google.common.base.Function;
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.commons.LongUtils;
import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType;
import org.jetbrains.annotations.NotNull;
@@ -44,14 +45,16 @@ final class AuthorizableIterator implements Iterator<Authorizable> {
private final long size;
private final Set<String> servedIds;

static AuthorizableIterator create(Iterator<String> authorizableOakPaths,
UserManagerImpl userManager,
AuthorizableType authorizableType) {
Iterator<Authorizable> it = Iterators.transform(authorizableOakPaths, new PathToAuthorizable(userManager, authorizableType));
long size = getSize(authorizableOakPaths);
@NotNull
static AuthorizableIterator create(@NotNull Iterator<Tree> authorizableTrees,
@NotNull UserManagerImpl userManager,
@NotNull AuthorizableType authorizableType) {
Iterator<Authorizable> it = Iterators.transform(authorizableTrees, new TreeToAuthorizable(userManager, authorizableType));
long size = getSize(authorizableTrees);
return new AuthorizableIterator(it, size, false);
}

@NotNull
static AuthorizableIterator create(boolean filterDuplicates, @NotNull Iterator<? extends Authorizable> it1, @NotNull Iterator<? extends Authorizable> it2) {
long size = 0;
for (Iterator<?> it : new Iterator[] {it1, it2}) {
@@ -118,25 +121,25 @@ private static long getSize(Iterator<?> it) {
}
}

private static class PathToAuthorizable implements Function<String, Authorizable> {
private static class TreeToAuthorizable implements Function<Tree, Authorizable> {

private final UserManagerImpl userManager;
private final Predicate<Authorizable> predicate;

PathToAuthorizable(UserManagerImpl userManager, AuthorizableType type) {
TreeToAuthorizable(UserManagerImpl userManager, AuthorizableType type) {
this.userManager = userManager;
this.predicate = new AuthorizableTypePredicate(type);
}

@Override
public Authorizable apply(String oakPath) {
public Authorizable apply(Tree tree) {
try {
Authorizable a = userManager.getAuthorizableByOakPath(oakPath);
Authorizable a = userManager.getAuthorizable(tree);
if (predicate.test(a)) {
return a;
}
} catch (RepositoryException e) {
log.debug("Failed to access authorizable {}", oakPath);
log.debug("Failed to access authorizable {}", tree.getPath());
}
return null;
}
@@ -217,12 +217,12 @@ private Iterator<Authorizable> getMembers(boolean includeInherited) throws Repos
}

// dynamic membership didn't cover all members -> extract from group-tree
Iterator<String> oakPaths = getMembershipProvider().getMembers(getTree(), includeInherited);
if (!oakPaths.hasNext()) {
Iterator<Tree> trees = getMembershipProvider().getMembers(getTree(), includeInherited);
if (!trees.hasNext()) {
return dynamicMembers;
}

AuthorizableIterator members = AuthorizableIterator.create(oakPaths, userMgr, AuthorizableType.AUTHORIZABLE);
AuthorizableIterator members = AuthorizableIterator.create(trees, userMgr, AuthorizableType.AUTHORIZABLE);
AuthorizableIterator allMembers = AuthorizableIterator.create(true, dynamicMembers, members);
return new RangeIteratorAdapter(allMembers, allMembers.getSize());
}

0 comments on commit 5a1a902

Please sign in to comment.