Implement ancestor axis in SQL

Currenty ancestor axis is implemented in Java using regex,
which then sends a second SQL query to fetch ancestors.
Implementing ancestor axis in SQL is more efficient, and is
needed for limiting/paginating Cps Path Query results.

Issue-ID: CPS-2416
Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech>
Change-Id: I0d8933f86c5a422f366ad7c417a17e263a13960f

diff --git a/cps-ri/src/main/java/org/onap/cps/ri/CpsDataPersistenceServiceImpl.java b/cps-ri/src/main/java/org/onap/cps/ri/CpsDataPersistenceServiceImpl.java
index bdbdc7c..cacdba9 100644 (file)
--- a/cps-ri/src/main/java/org/onap/cps/ri/CpsDataPersistenceServiceImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/ri/CpsDataPersistenceServiceImpl.java
@@ -39,8 +39,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -81,8 +79,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
     private final JsonObjectMapper jsonObjectMapper;
     private final SessionManager sessionManager;
 
-    private static final String REG_EX_FOR_OPTIONAL_LIST_INDEX = "(\\[@.+?])?)";
-
     @Override
     public void storeDataNodes(final String dataspaceName, final String anchorName,
                                final Collection<DataNode> dataNodes) {
@@ -228,13 +224,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
                                          final FetchDescendantsOption fetchDescendantsOption) {
         final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName);
         final CpsPathQuery cpsPathQuery = getCpsPathQuery(cpsPath);
-
-        Collection<FragmentEntity> fragmentEntities;
-        fragmentEntities = fragmentRepository.findByAnchorAndCpsPath(anchorEntity, cpsPathQuery);
-        if (cpsPathQuery.hasAncestorAxis()) {
-            final Collection<String> ancestorXpaths = processAncestorXpath(fragmentEntities, cpsPathQuery);
-            fragmentEntities = fragmentRepository.findByAnchorAndXpathIn(anchorEntity, ancestorXpaths);
-        }
+        final Collection<FragmentEntity> fragmentEntities =
+                fragmentRepository.findByAnchorAndCpsPath(anchorEntity, cpsPathQuery);
         return createDataNodesFromFragmentEntities(fetchDescendantsOption, fragmentEntities);
     }
 
@@ -246,7 +237,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
                                                       final PaginationOption paginationOption) {
         final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName);
         final CpsPathQuery cpsPathQuery = getCpsPathQuery(cpsPath);
-
         final List<Long> anchorIds;
         if (paginationOption == NO_PAGINATION) {
             anchorIds = Collections.emptyList();
@@ -256,17 +246,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
                 return Collections.emptyList();
             }
         }
-        Collection<FragmentEntity> fragmentEntities =
-            fragmentRepository.findByDataspaceAndCpsPath(dataspaceEntity, cpsPathQuery, anchorIds);
-
-        if (cpsPathQuery.hasAncestorAxis()) {
-            final Collection<String> ancestorXpaths = processAncestorXpath(fragmentEntities, cpsPathQuery);
-            if (anchorIds.isEmpty()) {
-                fragmentEntities = fragmentRepository.findByDataspaceAndXpathIn(dataspaceEntity, ancestorXpaths);
-            } else {
-                fragmentEntities = fragmentRepository.findByAnchorIdsAndXpathIn(anchorIds, ancestorXpaths);
-            }
-        }
+        final Collection<FragmentEntity> fragmentEntities =
+                fragmentRepository.findByDataspaceAndCpsPath(dataspaceEntity, cpsPathQuery, anchorIds);
         return createDataNodesFromFragmentEntities(fetchDescendantsOption, fragmentEntities);
     }
 
@@ -668,21 +649,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
                 .collect(Collectors.toMap(FragmentEntity::getXpath, fragmentEntity -> fragmentEntity));
     }
 
-    private static Set<String> processAncestorXpath(final Collection<FragmentEntity> fragmentEntities,
-                                                    final CpsPathQuery cpsPathQuery) {
-        final Set<String> ancestorXpath = new HashSet<>();
-        final Pattern pattern =
-            Pattern.compile("(.*/" + Pattern.quote(cpsPathQuery.getAncestorSchemaNodeIdentifier())
-                + REG_EX_FOR_OPTIONAL_LIST_INDEX + "/.*");
-        for (final FragmentEntity fragmentEntity : fragmentEntities) {
-            final Matcher matcher = pattern.matcher(fragmentEntity.getXpath());
-            if (matcher.matches()) {
-                ancestorXpath.add(matcher.group(1));
-            }
-        }
-        return ancestorXpath;
-    }
-
     private static boolean isRootXpath(final String xpath) {
         return "/".equals(xpath) || "".equals(xpath);
     }

diff --git a/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentQueryBuilder.java b/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentQueryBuilder.java
index b8bbf59..e35440e 100644 (file)
--- a/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentQueryBuilder.java
+++ b/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentQueryBuilder.java
@@ -30,8 +30,10 @@ import java.util.List;
 import java.util.Map;
 import java.util.Queue;
 import lombok.RequiredArgsConstructor;
+import org.apache.commons.text.StringSubstitutor;
 import org.onap.cps.cpspath.parser.CpsPathPrefixType;
 import org.onap.cps.cpspath.parser.CpsPathQuery;
+import org.onap.cps.cpspath.parser.CpsPathUtil;
 import org.onap.cps.ri.models.AnchorEntity;
 import org.onap.cps.ri.models.DataspaceEntity;
 import org.onap.cps.ri.models.FragmentEntity;
@@ -43,6 +45,7 @@ import org.springframework.stereotype.Component;
 @RequiredArgsConstructor
 @Component
 public class FragmentQueryBuilder {
+    private static final String DESCENDANT_PATH = "//";
 
     @PersistenceContext
     private EntityManager entityManager;
@@ -58,9 +61,10 @@ public class FragmentQueryBuilder {
         final StringBuilder sqlStringBuilder = new StringBuilder();
         final Map<String, Object> queryParameters = new HashMap<>();
 
-        sqlStringBuilder.append("SELECT fragment.* FROM fragment");
+        addSearchPrefix(cpsPathQuery, sqlStringBuilder);
         addWhereClauseForAnchor(anchorEntity, sqlStringBuilder, queryParameters);
         addNodeSearchConditions(cpsPathQuery, sqlStringBuilder, queryParameters, false);
+        addSearchSuffix(cpsPathQuery, sqlStringBuilder, queryParameters);
 
         return getQuery(sqlStringBuilder.toString(), queryParameters, FragmentEntity.class);
     }
@@ -78,13 +82,14 @@ public class FragmentQueryBuilder {
         final StringBuilder sqlStringBuilder = new StringBuilder();
         final Map<String, Object> queryParameters = new HashMap<>();
 
-        sqlStringBuilder.append("SELECT fragment.* FROM fragment");
+        addSearchPrefix(cpsPathQuery, sqlStringBuilder);
         if (anchorIdsForPagination.isEmpty()) {
             addWhereClauseForDataspace(dataspaceEntity, sqlStringBuilder, queryParameters);
         } else {
             addWhereClauseForAnchorIds(anchorIdsForPagination, sqlStringBuilder, queryParameters);
         }
         addNodeSearchConditions(cpsPathQuery, sqlStringBuilder, queryParameters, true);
+        addSearchSuffix(cpsPathQuery, sqlStringBuilder, queryParameters);
 
         return getQuery(sqlStringBuilder.toString(), queryParameters, FragmentEntity.class);
     }
@@ -143,7 +148,8 @@ public class FragmentQueryBuilder {
                                                 final Map<String, Object> queryParameters,
                                                 final boolean acrossAnchors) {
         addAbsoluteParentXpathSearchCondition(cpsPathQuery, sqlStringBuilder, queryParameters, acrossAnchors);
-        addXpathSearchCondition(cpsPathQuery, sqlStringBuilder, queryParameters);
+        sqlStringBuilder.append(" AND ");
+        addXpathSearchCondition(cpsPathQuery, sqlStringBuilder, queryParameters, "baseXpath");
         addLeafConditions(cpsPathQuery, sqlStringBuilder);
         addTextFunctionCondition(cpsPathQuery, sqlStringBuilder, queryParameters);
         addContainsFunctionCondition(cpsPathQuery, sqlStringBuilder, queryParameters);
@@ -151,13 +157,35 @@ public class FragmentQueryBuilder {
 
     private static void addXpathSearchCondition(final CpsPathQuery cpsPathQuery,
                                                 final StringBuilder sqlStringBuilder,
-                                                final Map<String, Object> queryParameters) {
-        sqlStringBuilder.append(" AND (xpath LIKE :escapedXpath OR "
-                + "(xpath LIKE :escapedXpath||'[@%]' AND xpath NOT LIKE :escapedXpath||'[@%]/%[@%]'))");
+                                                final Map<String, Object> queryParameters,
+                                                final String parameterName) {
+        queryParameters.put(parameterName, escapeXpathForSqlLike(cpsPathQuery));
+        final String sqlForXpathLikeContainerOrList = """
+                (
+                  (xpath LIKE :${xpathParamName})
+                  OR
+                  (xpath LIKE :${xpathParamName}||'[@%]' AND xpath NOT LIKE :${xpathParamName}||'[@%]/%[@%]')
+                )
+                """;
+        sqlStringBuilder.append(substitute(sqlForXpathLikeContainerOrList, Map.of("xpathParamName", parameterName)));
+    }
+
+    /**
+     * Returns a pattern suitable for use in an SQL LIKE expression, matching the xpath (absolute or descendant).
+     * For an absolute path such as "/bookstore/categories[@name='10% off']",
+     *  the output would be "/bookstore/categories[@name='10\% off']".
+     * For a descendant path such as "//categories[@name='10% off']",
+     *  the output would be "%/categories[@name='10\% off']".
+     * Note: percent sign '%' means match anything in SQL LIKE, while underscore '_' means match any single character.
+     *
+     * @param cpsPathQuery Cps Path Query
+     * @return a pattern suitable for use in an SQL LIKE expression.
+     */
+    private static String escapeXpathForSqlLike(final CpsPathQuery cpsPathQuery) {
         if (CpsPathPrefixType.ABSOLUTE.equals(cpsPathQuery.getCpsPathPrefixType())) {
-            queryParameters.put("escapedXpath", EscapeUtils.escapeForSqlLike(cpsPathQuery.getXpathPrefix()));
+            return EscapeUtils.escapeForSqlLike(cpsPathQuery.getXpathPrefix());
         } else {
-            queryParameters.put("escapedXpath", "%/" + EscapeUtils.escapeForSqlLike(cpsPathQuery.getDescendantName()));
+            return "%/" + EscapeUtils.escapeForSqlLike(cpsPathQuery.getDescendantName());
         }
     }
 
@@ -261,6 +289,55 @@ public class FragmentQueryBuilder {
         }
     }
 
+    private static void addSearchPrefix(final CpsPathQuery cpsPathQuery, final StringBuilder sqlStringBuilder) {
+        if (cpsPathQuery.hasAncestorAxis()) {
+            sqlStringBuilder.append("""
+                WITH RECURSIVE ancestors AS (
+                    SELECT parentFragment.* FROM fragment parentFragment
+                    WHERE parentFragment.id IN (
+                        SELECT parent_id FROM fragment""");
+        } else {
+            sqlStringBuilder.append("SELECT fragment.* FROM fragment");
+        }
+    }
+
+    private static void addSearchSuffix(final CpsPathQuery cpsPathQuery,
+                                        final StringBuilder sqlStringBuilder,
+                                        final Map<String, Object> queryParameters) {
+        if (cpsPathQuery.hasAncestorAxis()) {
+            sqlStringBuilder.append("""
+                          )
+                          UNION
+                            SELECT fragment.*
+                            FROM fragment
+                            JOIN ancestors ON ancestors.parent_id = fragment.id
+                        )
+                        SELECT * FROM ancestors
+                        WHERE""");
+
+            final String ancestorPath = DESCENDANT_PATH + cpsPathQuery.getAncestorSchemaNodeIdentifier();
+            final CpsPathQuery ancestorCpsPathQuery = CpsPathUtil.getCpsPathQuery(ancestorPath);
+            addAncestorNodeSearchCondition(ancestorCpsPathQuery, sqlStringBuilder, queryParameters);
+        }
+    }
+
+    private static void addAncestorNodeSearchCondition(final CpsPathQuery ancestorCpsPathQuery,
+                                                       final StringBuilder sqlStringBuilder,
+                                                       final Map<String, Object> queryParameters) {
+        if (ancestorCpsPathQuery.hasLeafConditions()) {
+            final String pathWithoutSlashes = ancestorCpsPathQuery.getNormalizedXpath().substring(2);
+            queryParameters.put("ancestorXpath", "%/" + EscapeUtils.escapeForSqlLike(pathWithoutSlashes));
+            sqlStringBuilder.append(" xpath LIKE :ancestorXpath");
+        } else {
+            addXpathSearchCondition(ancestorCpsPathQuery, sqlStringBuilder, queryParameters, "ancestorXpath");
+        }
+    }
+
+    private static <V> String substitute(final String template, final Map<String, V> valueMap) {
+        final StringSubstitutor stringSubstitutor = new StringSubstitutor(valueMap);
+        return stringSubstitutor.replace(template);
+    }
+
     private static void setQueryParameters(final Query query, final Map<String, Object> queryParameters) {
         for (final Map.Entry<String, Object> queryParameter : queryParameters.entrySet()) {
             query.setParameter(queryParameter.getKey(), queryParameter.getValue());

diff --git a/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentRepository.java b/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentRepository.java
index a8c1fd2..d95d322 100755 (executable)
--- a/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentRepository.java
+++ b/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentRepository.java
@@ -26,7 +26,6 @@ package org.onap.cps.ri.repository;
 import java.util.Collection;
 import java.util.List;
 import org.onap.cps.ri.models.AnchorEntity;
-import org.onap.cps.ri.models.DataspaceEntity;
 import org.onap.cps.ri.models.FragmentEntity;
 import org.onap.cps.ri.utils.EscapeUtils;
 import org.springframework.data.jpa.repository.JpaRepository;
@@ -63,21 +62,6 @@ public interface FragmentRepository extends JpaRepository<FragmentEntity, Long>,
         return findListByAnchorIdAndEscapedXpath(anchorEntity.getId(), escapedXpath);
     }
 
-    @Query(value = "SELECT fragment.* FROM fragment JOIN anchor ON anchor.id = fragment.anchor_id "
-        + "WHERE dataspace_id = :dataspaceId AND xpath IN (:xpaths)", nativeQuery = true)
-    List<FragmentEntity> findByDataspaceIdAndXpathIn(@Param("dataspaceId") int dataspaceId,
-                                                     @Param("xpaths") Collection<String> xpaths);
-
-    default List<FragmentEntity> findByDataspaceAndXpathIn(final DataspaceEntity dataspaceEntity,
-                                                           final Collection<String> xpaths) {
-        return findByDataspaceIdAndXpathIn(dataspaceEntity.getId(), xpaths);
-    }
-
-    @Query(value = "SELECT * FROM fragment WHERE anchor_id IN (:anchorIds)"
-            + " AND xpath IN (:xpaths)", nativeQuery = true)
-    List<FragmentEntity> findByAnchorIdsAndXpathIn(@Param("anchorIds") Collection<Long> anchorIds,
-                                                   @Param("xpaths") Collection<String> xpaths);
-
     @Modifying
     @Query(value = "DELETE FROM fragment WHERE anchor_id IN (:anchorIds)", nativeQuery = true)
     void deleteByAnchorIdIn(@Param("anchorIds") Collection<Long> anchorIds);

diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/functional/cps/QueryServiceIntegrationSpec.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/functional/cps/QueryServiceIntegrationSpec.groovy
index 5c2a4fc..3b49cfc 100644 (file)
--- a/integration-test/src/test/groovy/org/onap/cps/integration/functional/cps/QueryServiceIntegrationSpec.groovy
+++ b/integration-test/src/test/groovy/org/onap/cps/integration/functional/cps/QueryServiceIntegrationSpec.groovy
@@ -224,6 +224,7 @@ class QueryServiceIntegrationSpec extends FunctionalSpecBase {
             'ancestor with parent that does not exist'  | '//books/ancestor::parentDoesNoExist/categories'      || []
             'ancestor does not exist'                   | '//books/ancestor::ancestorDoesNotExist'              || []
             'ancestor combined with contains condition' | '//books[contains(@title,"Mat")]/ancestor::bookstore' || ["/bookstore"]
+            'ancestor is the same as search target'     | '//books/ancestor::books'                             || []
     }
 
     def 'Query for attribute by cps path of type ancestor with #scenario descendants.'() {