diff --git a/accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/helper/QueryHelper.java b/accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/helper/QueryHelper.java index 106be229..aec7bb83 100644 --- a/accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/helper/QueryHelper.java +++ b/accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/helper/QueryHelper.java @@ -48,12 +48,16 @@ import com.fasterxml.jackson.databind.ObjectMapper; public class QueryHelper { + public static final Logger LOG = LoggerFactory.getLogger(QueryHelper.class); private static final String ROOT_REP_POLICY_NODE = "/rep:policy"; private static final String ROOT_REPO_POLICY_NODE = "/" + Constants.REPO_POLICY_NODE; private static final String HOME_REP_POLICY = "/home/rep:policy"; + private static final String EXPLAIN_QUERY_FOR_ACL_INDEX = "EXPLAIN MEASURE SELECT * FROM [rep:ACL] AS s WHERE ISDESCENDANTNODE([/])"; + private static final String KEY_COST_PER_EXECUTION = "perExecution"; + /** every query cost below that threshold means a dedicated index exists, above that threshold means: fallback to traversal */ private static final double COST_THRESHOLD_FOR_QUERY_INDEX = 100d; @@ -110,8 +114,8 @@ public static Set getRepPolicyNodePaths(final Session session, boolean indexForRepACLExists = false; try { indexForRepACLExists = hasQueryIndexForACLs(session); - } catch(IOException|RepositoryException e) { - LOG.warn("Cannot figure out if query index for rep:ACL nodes exist", e); + } catch(Exception e) { + LOG.warn("Could not detect if query index for rep:ACL nodes exists: {}", e.getMessage(), e); } LOG.debug("Index for repACL exists: {}",indexForRepACLExists); String queryForAClNodes = indexForRepACLExists ? @@ -139,13 +143,14 @@ public static Set getRepPolicyNodePaths(final Session session, return paths; } - static boolean hasQueryIndexForACLs(final Session session) throws RepositoryException, IOException { - Query query = session.getWorkspace().getQueryManager().createQuery("EXPLAIN MEASURE SELECT * FROM [rep:ACL] AS s WHERE ISDESCENDANTNODE([/])", Query.JCR_SQL2); + public static boolean hasQueryIndexForACLs(final Session session) throws RepositoryException, IOException { + Query query = session.getWorkspace().getQueryManager().createQuery(EXPLAIN_QUERY_FOR_ACL_INDEX, Query.JCR_SQL2); QueryResult queryResult = query.execute(); Row row = queryResult.getRows().nextRow(); // inspired by https://github.com/apache/jackrabbit-oak/blob/cc8adb42d89bc4625138a62ab074e7794a4d39ab/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/query/QueryTest.java#L1092 String plan = row.getValue("plan").getString(); - String costJsonStr = plan.substring(plan.lastIndexOf('{')); + LOG.debug("Execution plan and cost for {}: {}", EXPLAIN_QUERY_FOR_ACL_INDEX, plan); + String costJsonStr = StringUtils.substringAfter(plan, "cost:"); double cost = getCostFromJsonStr(costJsonStr); // look at https://jackrabbit.apache.org/oak/docs/query/query-engine.html#cost-calculation for the threshold @@ -163,9 +168,12 @@ static double getCostFromJsonStr(String jsonStr) throws JsonProcessingException, // read the json strings and convert it into JsonNode JsonNode node = mapper.readTree(jsonStr); - + JsonNode sNode = node.get("s"); - double cost = sNode.isContainerNode() ? sNode.get("perExecution").asDouble(Double.MAX_VALUE) : sNode.asDouble(Double.MAX_VALUE); + if(sNode == null || (sNode.isContainerNode() && sNode.get(KEY_COST_PER_EXECUTION) == null)) { + throw new IllegalArgumentException("Unexpected json structure for query cost: " + jsonStr); + } + double cost = sNode.isContainerNode() ? sNode.get(KEY_COST_PER_EXECUTION).asDouble(Double.MAX_VALUE) : sNode.asDouble(Double.MAX_VALUE); return cost; } diff --git a/accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/helper/QueryHelperTest.java b/accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/helper/QueryHelperTest.java index 5985a0fc..a0c01aa5 100644 --- a/accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/helper/QueryHelperTest.java +++ b/accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/helper/QueryHelperTest.java @@ -9,6 +9,7 @@ package biz.netcentric.cq.tools.actool.helper; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; @@ -22,6 +23,7 @@ public class QueryHelperTest { public void testGetCostFromJsonScalar() throws JsonProcessingException, IOException { assertEquals(189540, QueryHelper.getCostFromJsonStr("{ \"s\": 189540.0 }")); + assertEquals(1, QueryHelper.getCostFromJsonStr("{ \"s\": 1.0 }")); } @Test @@ -29,7 +31,22 @@ public void testGetCostFromJsonObject() throws JsonProcessingException, IOExcept // the keys in sub object happen to be unquoted, the impl needs to use JsonParser.Feature.ALLOW_UNQUOTED_FIELD_NAMES assertEquals(1, QueryHelper.getCostFromJsonStr("{ \"s\": { perEntry: 1.0, perExecution: 1.0, count: 47814 } }")); - + assertEquals(189540, QueryHelper.getCostFromJsonStr("{ \"s\": { perEntry: 1.0, perExecution: 189540.0, count: 47814 } }")); + } + @Test + public void testInvalidCostJson() throws JsonProcessingException, IOException { + + IllegalArgumentException thrown = assertThrows( + IllegalArgumentException.class, + () -> { + QueryHelper.getCostFromJsonStr("{ \"s\": { someChangedFormat: 123 } }"); + }, + "Expected getCostFromJsonStr() to throw an IllegalArgumentException for invalid JSON" + ); + assertEquals("Unexpected json structure for query cost: { \"s\": { someChangedFormat: 123 } }", thrown.getMessage()); + + } + }