Skip to content
Permalink
Browse files
OAK-9737 : Avoid duplicate tree resolution by using ResultRow.getTree
  • Loading branch information
anchela committed Mar 24, 2022
1 parent 871ce1b commit 5253061d1478f13db2d9481be0b3d45c14518632
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 55 deletions.
@@ -133,13 +133,17 @@ public Tree getTree(String identifier) {

checkArgument(UUIDUtils.isValidUUID(uuid), "Not a valid identifier '" + identifier + '\'');

String basePath = resolveUUID(uuid);
if (basePath == null) {
Tree tree = resolveUUIDToTree(createPropertyValue(uuid));
if (tree == null) {
return null;
} else if (k == -1) {
return root.getTree(basePath);
return tree;
} else {
return root.getTree(PathUtils.concat(basePath, identifier.substring(k + 1)));
Iterable<String> subpath = PathUtils.elements(identifier.substring(k + 1));
for (String element : subpath) {
tree = tree.getChild(element);
}
return tree;
}
}
}
@@ -171,7 +175,7 @@ public String getPath(String identifier) {
public String getPath(PropertyState referenceValue) {
int type = referenceValue.getType().tag();
if (type == PropertyType.REFERENCE || type == PropertyType.WEAKREFERENCE) {
return resolveUUID(referenceValue);
return resolveUUID(PropertyValues.create(referenceValue));
} else {
throw new IllegalArgumentException("Invalid value type");
}
@@ -338,14 +342,17 @@ public boolean apply(String path) {

@Nullable
public String resolveUUID(String uuid) {
return resolveUUID(StringPropertyState.stringProperty("", uuid));
return resolveUUID(createPropertyValue(uuid));
}

private String resolveUUID(PropertyState uuid) {
return resolveUUID(PropertyValues.create(uuid));

@Nullable
private String resolveUUID(@NotNull PropertyValue uuid) {
Tree tree = resolveUUIDToTree(uuid);
return (tree == null) ? null : tree.getPath();
}

private String resolveUUID(PropertyValue uuid) {
@Nullable
private Tree resolveUUIDToTree(@NotNull PropertyValue uuid) {
try {
Map<String, PropertyValue> bindings = Collections.singletonMap("id", uuid);
Result result = root.getQueryEngine().executeQuery(
@@ -355,20 +362,24 @@ private String resolveUUID(PropertyValue uuid) {
Query.JCR_SQL2,
bindings, NO_MAPPINGS);

String path = null;
Tree tree = null;
for (ResultRow rr : result.getRows()) {
if (path != null) {
log.error("multiple results for identifier lookup: " + path + " vs. " + rr.getPath());
if (tree != null) {
log.error("multiple results for identifier lookup: " + tree.getPath() + " vs. " + rr.getPath());
return null;
} else {
path = rr.getPath();
tree = rr.getTree(null);
}
}
return path;
return tree;
} catch (ParseException ex) {
log.error("query failed", ex);
return null;
}
}


@NotNull
private static PropertyValue createPropertyValue(@NotNull String uuid) {
return PropertyValues.create(StringPropertyState.stringProperty("", uuid));
}
}
@@ -375,10 +375,11 @@ public AccessControlPolicy[] getEffectivePolicies(@NotNull Set<Principal> princi
Set<String> processed = Sets.newHashSet();
Predicate<Tree> predicate = new PrincipalPredicate(principals);
for (ResultRow row : aceResult.getRows()) {
String acePath = row.getPath();
Tree aceTree = row.getTree(null);
String acePath = aceTree.getPath();
String aclName = Text.getName(Text.getRelativeParent(acePath, 1));

Tree accessControlledTree = r.getTree(Text.getRelativeParent(acePath, 2));
Tree accessControlledTree = aceTree.getParent().getParent();
if (!POLICY_NODE_NAMES.contains(aclName) || !accessControlledTree.exists()) {
log.debug("Isolated access control entry -> ignore query result at {}", acePath);
continue;
@@ -488,7 +489,7 @@ private JackrabbitAccessControlList createPrincipalACL(@Nullable String oakPath,
List<ACE> entries = new ArrayList<>();
Map<String, Principal> principalMap = new HashMap<>();
for (ResultRow row : aceResult.getRows()) {
Tree aceTree = root.getTree(row.getPath());
Tree aceTree = row.getTree(null);
if (Util.isACE(aceTree, ntMgr)) {
String aclPath = Text.getRelativeParent(aceTree.getPath(), 1);
String path;
@@ -240,8 +240,7 @@ Tree getAuthorizableByPrincipal(@NotNull Principal principal) {

Iterator<? extends ResultRow> rows = result.getRows().iterator();
if (rows.hasNext()) {
String path = rows.next().getPath();
return root.getTree(path);
return rows.next().getTree(null);
}
} catch (ParseException ex) {
log.error("Failed to retrieve authorizable by principal", ex);
@@ -16,8 +16,6 @@
*/
package org.apache.jackrabbit.oak.security.user.query;

import javax.jcr.RepositoryException;

import com.google.common.base.Function;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.oak.api.ResultRow;
@@ -33,6 +31,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.jcr.RepositoryException;

/**
* Function to convert query result rows {@link Authorizable}s of a given
* target type.
@@ -44,12 +44,14 @@ class ResultRowToAuthorizable implements Function<ResultRow, Authorizable> {
private final UserManagerImpl userManager;
private final Root root;
private final AuthorizableType targetType;
private final String selectorName;

ResultRowToAuthorizable(@NotNull UserManagerImpl userManager, @NotNull Root root,
@Nullable AuthorizableType targetType) {
@Nullable AuthorizableType targetType, @NotNull String[] selectorNames) {
this.userManager = userManager;
this.root = root;
this.targetType = (targetType == null || AuthorizableType.AUTHORIZABLE == targetType) ? null : targetType;
this.selectorName = (selectorNames.length == 0) ? null : selectorNames[0];
}

@Nullable
@@ -63,9 +65,11 @@ public Authorizable apply(@Nullable ResultRow row) {
private Authorizable getAuthorizable(@Nullable ResultRow row) {
Authorizable authorizable = null;
if (row != null) {
String resultPath = row.getValue(QueryConstants.JCR_PATH).getValue(Type.STRING);
Tree tree = row.getTree(selectorName);
if (!tree.exists()) {
return null;
}
try {
Tree tree = root.getTree(resultPath);
AuthorizableType type = UserUtil.getType(tree);
while (tree.exists() && !tree.isRoot() && type == null) {
tree = tree.getParent();
@@ -75,7 +79,7 @@ private Authorizable getAuthorizable(@Nullable ResultRow row) {
authorizable = userManager.getAuthorizable(tree);
}
} catch (RepositoryException e) {
log.debug("Failed to access authorizable {}", resultPath);
log.debug("Failed to access authorizable {}", row.getPath());
}
}
return authorizable;
@@ -273,7 +273,7 @@ private Iterator<Authorizable> findAuthorizables(@NotNull String statement,
statement, javax.jcr.query.Query.XPATH, limit, offset,
NO_BINDINGS, namePathMapper.getSessionLocalMappings());
Iterable<? extends ResultRow> resultRows = query.getRows();
Iterator<Authorizable> authorizables = Iterators.transform(resultRows.iterator(), new ResultRowToAuthorizable(userManager, root, type));
Iterator<Authorizable> authorizables = Iterators.transform(resultRows.iterator(), new ResultRowToAuthorizable(userManager, root, type, query.getSelectorNames()));
return Iterators.filter(authorizables, new UniqueResultPredicate());
} catch (ParseException e) {
throw new RepositoryException("Invalid user query "+statement, e);
@@ -1745,12 +1745,7 @@ public void testEffectivePolicyIsolatedAce() throws Exception {
ace.setProperty(REP_PRINCIPAL_NAME, testPrincipal.getName());
ace.setProperty(REP_PRIVILEGES, ImmutableList.of(JCR_READ), Type.NAMES);

ResultRow row = when(mock(ResultRow.class).getPath()).thenReturn(ace.getPath()).getMock();
Iterable rows = ImmutableList.of(row);
Result res = mock(Result.class);
when(res.getRows()).thenReturn(rows).getMock();
QueryEngine qe = mock(QueryEngine.class);
when(qe.executeQuery(anyString(), anyString(), any(Map.class), any(Map.class))).thenReturn(res);
QueryEngine qe = mockQueryEngine(ace);
when(r.getQueryEngine()).thenReturn(qe);

AccessControlManagerImpl mgr = createAccessControlManager(r, getNamePathMapper());
@@ -1770,18 +1765,23 @@ public void testEffectivePolicyWrongPrimaryType() throws Exception {
ace.setProperty(REP_PRINCIPAL_NAME, testPrincipal.getName());
ace.setProperty(REP_PRIVILEGES, ImmutableList.of(JCR_READ), Type.NAMES);

ResultRow row = when(mock(ResultRow.class).getPath()).thenReturn(ace.getPath()).getMock();
Iterable rows = ImmutableList.of(row);
Result res = mock(Result.class);
when(res.getRows()).thenReturn(rows).getMock();
QueryEngine qe = mock(QueryEngine.class);
when(qe.executeQuery(anyString(), anyString(), any(Map.class), any(Map.class))).thenReturn(res);
QueryEngine qe = mockQueryEngine(ace);
when(r.getQueryEngine()).thenReturn(qe);

AccessControlManagerImpl mgr = createAccessControlManager(r, getNamePathMapper());
AccessControlPolicy[] policies = mgr.getEffectivePolicies(ImmutableSet.of(testPrincipal));
assertPolicies(policies, 0);
}

private static QueryEngine mockQueryEngine(@NotNull Tree aceTree) throws Exception {
ResultRow row = when(mock(ResultRow.class).getTree(null)).thenReturn(aceTree).getMock();
Iterable rows = ImmutableList.of(row);
Result res = mock(Result.class);
when(res.getRows()).thenReturn(rows).getMock();
QueryEngine qe = mock(QueryEngine.class);
when(qe.executeQuery(anyString(), anyString(), any(Map.class), any(Map.class))).thenReturn(res);
return qe;
}

@Test(expected = RepositoryException.class)
public void testEffectivePolicyFailingQuery() throws Exception {
@@ -19,16 +19,13 @@
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.User;
import org.apache.jackrabbit.oak.api.ContentSession;
import org.apache.jackrabbit.oak.api.PropertyValue;
import org.apache.jackrabbit.oak.api.ResultRow;
import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.plugins.memory.PropertyValues;
import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
import org.apache.jackrabbit.oak.security.user.AbstractUserTest;
import org.apache.jackrabbit.oak.security.user.UserManagerImpl;
import org.apache.jackrabbit.oak.spi.query.QueryConstants;
import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -61,12 +58,11 @@ public void before() throws Exception {
@NotNull
private ResultRowToAuthorizable createResultRowToAuthorizable(@NotNull Root r, @Nullable AuthorizableType targetType) {
UserManagerImpl umgr = createUserManagerImpl(r);
return new ResultRowToAuthorizable(umgr, r, targetType);
return new ResultRowToAuthorizable(umgr, r, targetType, new String[0]);
}

private static ResultRow createResultRow(@NotNull String path) {
PropertyValue propValue = PropertyValues.newPath(path);
return when(mock(ResultRow.class).getValue(QueryConstants.JCR_PATH)).thenReturn(propValue).getMock();
private static ResultRow createResultRow(@NotNull Tree tree) {
return when(mock(ResultRow.class).getTree(null)).thenReturn(tree).getMock();
}

@Test
@@ -76,20 +72,18 @@ public void testApplyNullRow() {

@Test
public void testRowToNonExistingTree() {
PropertyValue propValue = PropertyValues.newPath("/path/to/nonExisting/tree");
ResultRow row = when(mock(ResultRow.class).getValue(QueryConstants.JCR_PATH)).thenReturn(propValue).getMock();
assertNull(groupRrta.apply(row));
assertNull(groupRrta.apply(createResultRow(root.getTree("/path/to/nonExisting/tree"))));
}

@Test
public void testRowToRootTree() {
assertNull(groupRrta.apply(createResultRow(PathUtils.ROOT_PATH)));
assertNull(groupRrta.apply(createResultRow(root.getTree(PathUtils.ROOT_PATH))));
}

@Test
public void testRowToUserTree() throws Exception {
User user = getTestUser();
ResultRow row = createResultRow(user.getPath());
ResultRow row = createResultRow(root.getTree(user.getPath()));

assertNull(groupRrta.apply(row));

@@ -103,7 +97,7 @@ public void testRowToUserSubTree() throws Exception {
User user = getTestUser();
Tree t = root.getTree(user.getPath());
t = TreeUtil.addChild(t, "child", NT_OAK_UNSTRUCTURED);
ResultRow row = createResultRow(t.getPath());
ResultRow row = createResultRow(t);

assertNull(groupRrta.apply(row));

@@ -115,7 +109,8 @@ public void testRowToUserSubTree() throws Exception {
@Test
public void testRowToNonExistingUserSubTree() throws Exception {
User user = getTestUser();
ResultRow row = createResultRow(PathUtils.concat(user.getPath(), "child"));
Tree tree = root.getTree(user.getPath()).getChild("child");
ResultRow row = createResultRow(tree);

assertNull(userRrta.apply(row));
}
@@ -128,7 +123,7 @@ public void testRowNonAccessibleUserTree() throws Exception {
try (ContentSession cs = login(new SimpleCredentials(user.getID(), user.getID().toCharArray()))) {
Root r = cs.getLatestRoot();
ResultRowToAuthorizable rrta = createResultRowToAuthorizable(r, null);
assertNull(rrta.apply(createResultRow(userPath)));
assertNull(rrta.apply(createResultRow(r.getTree(userPath))));
}
}
}

0 comments on commit 5253061

Please sign in to comment.