You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2022/03/24 15:51:21 UTC
[jackrabbit-oak] branch OAK-9737 created (now 5253061)
This is an automated email from the ASF dual-hosted git repository.
angela pushed a change to branch OAK-9737
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git.
at 5253061 OAK-9737 : Avoid duplicate tree resolution by using ResultRow.getTree
This branch includes the following new commits:
new 5253061 OAK-9737 : Avoid duplicate tree resolution by using ResultRow.getTree
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[jackrabbit-oak] 01/01: OAK-9737 : Avoid duplicate tree resolution by using ResultRow.getTree
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch OAK-9737
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 5253061d1478f13db2d9481be0b3d45c14518632
Author: angela <an...@adobe.com>
AuthorDate: Thu Mar 24 16:50:59 2022 +0100
OAK-9737 : Avoid duplicate tree resolution by using ResultRow.getTree
---
.../oak/plugins/identifier/IdentifierManager.java | 43 ++++++++++++++--------
.../accesscontrol/AccessControlManagerImpl.java | 7 ++--
.../jackrabbit/oak/security/user/UserProvider.java | 3 +-
.../user/query/ResultRowToAuthorizable.java | 16 +++++---
.../oak/security/user/query/UserQueryManager.java | 2 +-
.../AccessControlManagerImplTest.java | 24 ++++++------
.../user/query/ResultRowToAuthorizableTest.java | 25 +++++--------
7 files changed, 65 insertions(+), 55 deletions(-)
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java
index 97223ee..2607917 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java
@@ -133,13 +133,17 @@ public class IdentifierManager {
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 class IdentifierManager {
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 class IdentifierManager {
@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 @@ public class IdentifierManager {
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));
+ }
}
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImpl.java
index f8804c3..249cfd4 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImpl.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImpl.java
@@ -375,10 +375,11 @@ public class AccessControlManagerImpl extends AbstractAccessControlManager imple
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 @@ public class AccessControlManagerImpl extends AbstractAccessControlManager imple
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;
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserProvider.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserProvider.java
index 5a6ba03..46008c5 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserProvider.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserProvider.java
@@ -240,8 +240,7 @@ class UserProvider extends AuthorizableBaseProvider {
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);
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizable.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizable.java
index d7e23a7..0c65c58 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizable.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizable.java
@@ -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.jetbrains.annotations.Nullable;
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 @@ class ResultRowToAuthorizable implements Function<ResultRow, Authorizable> {
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 @@ class ResultRowToAuthorizable implements Function<ResultRow, Authorizable> {
authorizable = userManager.getAuthorizable(tree);
}
} catch (RepositoryException e) {
- log.debug("Failed to access authorizable {}", resultPath);
+ log.debug("Failed to access authorizable {}", row.getPath());
}
}
return authorizable;
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java
index 4728d75..09adc15 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java
@@ -273,7 +273,7 @@ public class UserQueryManager {
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);
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java
index 26e2ed2..fc0a33e 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java
@@ -1745,12 +1745,7 @@ public class AccessControlManagerImplTest extends AbstractAccessControlTest impl
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 class AccessControlManagerImplTest extends AbstractAccessControlTest impl
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 {
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizableTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizableTest.java
index 7a25c62..050254c 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizableTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizableTest.java
@@ -19,16 +19,13 @@ package org.apache.jackrabbit.oak.security.user.query;
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 class ResultRowToAuthorizableTest extends AbstractUserTest {
@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 class ResultRowToAuthorizableTest extends AbstractUserTest {
@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 class ResultRowToAuthorizableTest extends AbstractUserTest {
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 class ResultRowToAuthorizableTest extends AbstractUserTest {
@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 class ResultRowToAuthorizableTest extends AbstractUserTest {
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))));
}
}
}
\ No newline at end of file