Improved unit tests for cost JSON interpretation

Include index usage in AC Tool log Fix JSON retrieval from plan
Netcentric · Jun 14, 2024 · 9f855a5 · 9f855a5
1 parent 1e12fbd
commit 9f855a5
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 8 deletions.
diff --git a/...sscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/helper/QueryHelper.java b/...sscontroltool-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<String> 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<String> 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/...ntroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/helper/QueryHelperTest.java b/...ntroltool-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,14 +23,30 @@ public class QueryHelperTest {
     public void testGetCostFromJsonScalar() throws JsonProcessingException, IOException {
 
         assertEquals(189540, QueryHelper.getCostFromJsonStr("{ \"s\": 189540.0 }"));
+        assertEquals(1, QueryHelper.getCostFromJsonStr("{ \"s\": 1.0 }"));
     }
 
     @Test
     public void testGetCostFromJsonObject() throws JsonProcessingException, IOException {
 
         // 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());
+
+    }
+
 }