Fix for quickfind with descendants incl. list entries

- add specialized regex
- fixed legacy issues with absolute paths (added ^ in regex's)
- split into 2 methods for normal queries and quickfind
- fixed order of private methods
- Most code 'cherry picked by hand' from https://gerrit.onap.org/r/c/cps/+/134481

Issue-ID: CPS-1671
Signed-off-by: ToineSiebelink <toine.siebelink@est.tech>
Change-Id: I37988ac24d9b8d4df7a249b79068910c15a6cd20

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
index 0a77de2..7a556bf 100644 (file)
--- a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
@@ -359,7 +359,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
                                                            final AnchorEntity anchorEntity,
                                                            final CpsPathQuery cpsPathQuery) {
         Collection<FragmentEntity> fragmentEntities;
-        final String xpathRegex = FragmentQueryBuilder.getXpathSqlRegex(cpsPathQuery, true);
+        final String xpathRegex = FragmentQueryBuilder.getXpathSqlRegexForQuickFindWithDescendants(cpsPathQuery);
         final List<FragmentExtract> fragmentExtracts =
             fragmentRepository.quickFindWithDescendants(anchorEntity.getId(), xpathRegex);
         fragmentEntities = FragmentEntityArranger.toFragmentEntityTrees(anchorEntity, fragmentExtracts);
@@ -381,7 +381,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
             final FetchDescendantsOption fetchDescendantsOption, final DataspaceEntity dataspaceEntity,
             final CpsPathQuery cpsPathQuery) {
         Collection<FragmentEntity> fragmentEntities;
-        final String xpathRegex = FragmentQueryBuilder.getXpathSqlRegex(cpsPathQuery, true);
+        final String xpathRegex = FragmentQueryBuilder.getXpathSqlRegexForQuickFindWithDescendants(cpsPathQuery);
         final List<FragmentExtract> fragmentExtracts = fragmentRepository
                 .quickFindWithDescendantsAcrossAnchor(dataspaceEntity.getId(), xpathRegex);
         final Map<Long, AnchorEntity> fragmentExtractAnchorMap = getFragmentExtractAnchorMap(fragmentExtracts);

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java
index 5716304..f124a8a 100644 (file)
--- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java
@@ -38,10 +38,10 @@ import org.springframework.stereotype.Component;
 @Slf4j
 @Component
 public class FragmentQueryBuilder {
-    private static final String REGEX_ABSOLUTE_PATH_PREFIX = ".*\\/";
-    private static final String REGEX_OPTIONAL_LIST_INDEX_POSTFIX = "(\\[@(?!.*\\[).*?])?";
-    private static final String REGEX_DESCENDANT_PATH_POSTFIX = "(\\/.*)?";
-    private static final String REGEX_END_OF_INPUT = "$";
+    private static final String REGEX_ABSOLUTE_PATH_PREFIX = "^";
+    private static final String REGEX_DESCENDANT_PATH_PREFIX = "^.*\\/";
+    private static final String REGEX_OPTIONAL_LIST_INDEX_POSTFIX = "(\\[@(?!.*\\[).*?])?$";
+    private static final String REGEX_FOR_QUICK_FIND_WITH_DESCENDANTS = "(\\[@.*?])?(\\/.*)?$";
 
     @PersistenceContext
     private EntityManager entityManager;
@@ -60,7 +60,7 @@ public class FragmentQueryBuilder {
         final Map<String, Object> queryParameters = new HashMap<>();
         queryParameters.put("anchorId", anchorId);
         sqlStringBuilder.append(" AND xpath ~ :xpathRegex");
-        final String xpathRegex = getXpathSqlRegex(cpsPathQuery, false);
+        final String xpathRegex = getXpathSqlRegex(cpsPathQuery);
         queryParameters.put("xpathRegex", xpathRegex);
         if (cpsPathQuery.hasLeafConditions()) {
             sqlStringBuilder.append(" AND attributes @> :leafDataAsJson\\:\\:jsonb");
@@ -84,7 +84,7 @@ public class FragmentQueryBuilder {
         final StringBuilder sqlStringBuilder = new StringBuilder("SELECT * FROM FRAGMENT WHERE dataspace_id = "
                 + ":dataspaceId AND xpath ~ :xpathRegex");
         final Map<String, Object> queryParameters = new HashMap<>();
-        final String xpathRegex = getXpathSqlRegex(cpsPathQuery, false);
+        final String xpathRegex = getXpathSqlRegex(cpsPathQuery);
         queryParameters.put("dataspaceId", dataspaceId);
         queryParameters.put("xpathRegex", xpathRegex);
         if (cpsPathQuery.hasLeafConditions()) {
@@ -100,26 +100,40 @@ public class FragmentQueryBuilder {
     }
 
     /**
-     * Create a regular expression (string) for xpath based on the given cps path query.
+     * Create a regular expression (string) for matching xpaths based on the given cps path query.
      *
-     * @param cpsPathQuery  the cps path query to determine the required regular expression
-     * @param includeDescendants include descendants yes or no
+     * @param cpsPathQuery the cps path query to determine the required regular expression
      * @return a string representing the required regular expression
      */
-    public static String getXpathSqlRegex(final CpsPathQuery cpsPathQuery, final boolean includeDescendants) {
+    public static String getXpathSqlRegex(final CpsPathQuery cpsPathQuery) {
+        final StringBuilder xpathRegexBuilder = getRegexStringBuilderWithPrefix(cpsPathQuery);
+        xpathRegexBuilder.append(REGEX_OPTIONAL_LIST_INDEX_POSTFIX);
+        return xpathRegexBuilder.toString();
+    }
+
+    /**
+     * Create a regular expression (string) for matching xpaths with (all) descendants
+     * based on the given cps path query.
+     *
+     * @param cpsPathQuery the cps path query to determine the required regular expression
+     * @return a string representing the required regular expression
+     */
+    public static String getXpathSqlRegexForQuickFindWithDescendants(final CpsPathQuery cpsPathQuery) {
+        final StringBuilder xpathRegexBuilder = getRegexStringBuilderWithPrefix(cpsPathQuery);
+        xpathRegexBuilder.append(REGEX_FOR_QUICK_FIND_WITH_DESCENDANTS);
+        return xpathRegexBuilder.toString();
+    }
+
+    private static StringBuilder getRegexStringBuilderWithPrefix(final CpsPathQuery cpsPathQuery) {
         final StringBuilder xpathRegexBuilder = new StringBuilder();
         if (CpsPathPrefixType.ABSOLUTE.equals(cpsPathQuery.getCpsPathPrefixType())) {
-            xpathRegexBuilder.append(escapeXpath(cpsPathQuery.getXpathPrefix()));
-        } else {
             xpathRegexBuilder.append(REGEX_ABSOLUTE_PATH_PREFIX);
-            xpathRegexBuilder.append(escapeXpath(cpsPathQuery.getDescendantName()));
-        }
-        xpathRegexBuilder.append(REGEX_OPTIONAL_LIST_INDEX_POSTFIX);
-        if (includeDescendants) {
-            xpathRegexBuilder.append(REGEX_DESCENDANT_PATH_POSTFIX);
+            xpathRegexBuilder.append(escapeXpath(cpsPathQuery.getXpathPrefix()));
+            return xpathRegexBuilder;
         }
-        xpathRegexBuilder.append(REGEX_END_OF_INPUT);
-        return xpathRegexBuilder.toString();
+        xpathRegexBuilder.append(REGEX_DESCENDANT_PATH_PREFIX);
+        xpathRegexBuilder.append(escapeXpath(cpsPathQuery.getDescendantName()));
+        return xpathRegexBuilder;
     }
 
     private static String escapeXpath(final String xpath) {

diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy
index 496b36d..4cfc082 100644 (file)
--- a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy
+++ b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy
@@ -23,6 +23,8 @@ package org.onap.cps.integration.functional
 import org.onap.cps.integration.base.FunctionalSpecBase
 import org.onap.cps.spi.FetchDescendantsOption
 
+import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS
+
 class CpsQueryServiceIntegrationSpec extends FunctionalSpecBase {
 
     def objectUnderTest
@@ -45,4 +47,21 @@ class CpsQueryServiceIntegrationSpec extends FunctionalSpecBase {
             'the and condition is used'                   | '//books[@lang="English" and @price=15]'   || 2                  | [lang:"English", price:15]
             'the and is used where result does not exist' | '//books[@lang="English" and @price=1000]' || 0                  | []
     }
+
+    def 'Cps Path queries with all descendants including descendants that are list entries: #scenario.'() {
+        when: 'a query is executed to get a data node by the given cps path'
+            def result = objectUnderTest.queryDataNodes(FUNCTIONAL_TEST_DATASPACE, BOOKSTORE_ANCHOR, cpsPath, INCLUDE_ALL_DESCENDANTS)
+        then: 'correct number of datanodes are returned'
+            assert countDataNodesInTree(result) == expectedNumberOfDataNodes
+        where: 'the following cps paths are used'
+            scenario                              | cpsPath                                 || expectedNumberOfDataNodes
+            'absolute path all list entries'      | '/bookstore/categories'                 || 7
+            'absolute path 1 list entry by key'   | '/bookstore/categories[@code="3"]'      || 2
+            'absolute path 1 list entry by leaf'  | '/bookstore/categories[@name="Comedy"]' || 2
+            'relative path all list entries'      | '//categories'                          || 7
+            'relative path 1 list entry by key'   | '//categories[@code="3"]'               || 2
+            'relative path 1 list entry by leaf'  | '//categories[@name="Comedy"]'          || 2
+            'incomplete absolute path'            | '/categories'                           || 0
+            'incomplete absolute path list entry' | '/categories[@code="3"]'                || 0
+    }
 }