Fix "ends-with" query syntax to conform with xPath definition

"ends-with" is HOW we resolve it in sql query. 'descendant anywhere' is the correct Path name for the '//' operator
- Updated method names, variable names, test description to reflect the correct terminolgy
- Udpated query to always perfix the target (descendant name) with an '\' so it alwasy only matches whole node names
- Updated regex for cpsPath to NOT accept triple /// (as per xPath this is invalid since a ndoeName cannot start with or contain a node separator

Issue-ID: CPS-336

Signed-off-by: ToineSiebelink <toine.siebelink@est.tech>
Change-Id: I9f181d6986d038311b839e3f9c5afc4237c7d347

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 26aa4ac..ca279f3 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
@@ -145,7 +145,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
                     .getLeafName(), cpsPathQuery.getLeafValue());
         } else {
             fragmentEntities = fragmentRepository
-                .getByAnchorAndEndsWithXpath(anchorEntity.getId(), cpsPathQuery.getEndsWith());
+                .getByAnchorAndXpathEndsInDescendantName(anchorEntity.getId(), cpsPathQuery.getDescendantName());
         }
         return fragmentEntities.stream()
             .map(fragmentEntity -> toDataNode(fragmentEntity, fetchDescendantsOption))

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java
index 97a304d..ce78c06 100644 (file)
--- a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java
@@ -35,7 +35,7 @@ public class CpsPathQuery {
     private String xpathPrefix;
     private String leafName;
     private Object leafValue;
-    private String endsWith;
+    private String descendantName;
 
     private static final String NON_CAPTURING_GROUP_1_TO_99_YANG_CONTAINERS = "((?:\\/[^\\/]+){1,99})";
 
@@ -45,7 +45,7 @@ public class CpsPathQuery {
     private static final Pattern QUERY_CPS_PATH_WITH_SINGLE_LEAF_PATTERN =
         Pattern.compile(NON_CAPTURING_GROUP_1_TO_99_YANG_CONTAINERS + YANG_LEAF_VALUE_EQUALS_CONDITION);
 
-    private static final Pattern QUERY_CPS_PATH_ENDS_WITH_PATTERN = Pattern.compile("\\/\\/(.+)");
+    private static final Pattern DESCENDANT_ANYWHERE_PATTERN = Pattern.compile("\\/\\/([^\\/].+)");
 
     private static final Pattern LEAF_INTEGER_VALUE_PATTERN = Pattern.compile("[-+]?\\d+");
 
@@ -67,10 +67,10 @@ public class CpsPathQuery {
             cpsPathQuery.setLeafValue(convertLeafValueToCorrectType(matcher.group(3), cpsPath));
             return cpsPathQuery;
         }
-        matcher = QUERY_CPS_PATH_ENDS_WITH_PATTERN.matcher(cpsPath);
+        matcher = DESCENDANT_ANYWHERE_PATTERN.matcher(cpsPath);
         if (matcher.matches()) {
-            cpsPathQuery.setCpsPathQueryType(CpsPathQueryType.XPATH_ENDS_WITH);
-            cpsPathQuery.setEndsWith(matcher.group(1));
+            cpsPathQuery.setCpsPathQueryType(CpsPathQueryType.XPATH_HAS_DESCENDANT_ANYWHERE);
+            cpsPathQuery.setDescendantName(matcher.group(1));
             return cpsPathQuery;
         }
         throw new CpsPathException("Invalid cps path.",

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQueryType.java b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQueryType.java
index 1a0f8ca..abdbfb9 100644 (file)
--- a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQueryType.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQueryType.java
@@ -24,9 +24,9 @@ package org.onap.cps.spi.query;
  */
 public enum CpsPathQueryType {
     /**
-     * Xpath ends with cps path query type e.g. //cps-path .
+     * Xpath descendant anywhere type e.g. //nodeName .
      */
-    XPATH_ENDS_WITH,
+    XPATH_HAS_DESCENDANT_ANYWHERE,
     /**
      * Xpath leaf value cps path query type e.g. /cps-path[@leafName=leafValue] .
      */

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java
index 5ff7cfc..eb2e82b 100755 (executable)
--- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java
@@ -1,6 +1,6 @@
 /*-\r
  * ============LICENSE_START=======================================================\r
- *  Copyright (C) 2020 Nordix Foundation. All rights reserved.\r
+ *  Copyright (C) 2020-201 Nordix Foundation. All rights reserved.\r
  *  Modifications Copyright (C) 2020 Bell Canada. All rights reserved.\r
  * ================================================================================\r
  * Licensed under the Apache License, Version 2.0 (the "License");\r
@@ -61,7 +61,9 @@ public interface FragmentRepository extends JpaRepository<FragmentEntity, Long>
     List<FragmentEntity> getByAnchorAndXpathAndLeafAttributes(@Param("anchor") int anchorId, @Param("xpath")\r
         String xpathPrefix, @Param("leafName") String leafName, @Param("leafValue") Object leafValue);\r
 \r
-    @Query(value = "SELECT * FROM FRAGMENT WHERE anchor_id = :anchor AND xpath LIKE %:xpath", nativeQuery = true)\r
-    // Above query will match the end of an xpath and anchor id\r
-    List<FragmentEntity> getByAnchorAndEndsWithXpath(@Param("anchor") int anchorId, @Param("xpath") String xpath);\r
-}
\ No newline at end of file
+    @Query(value = "SELECT * FROM FRAGMENT WHERE anchor_id = :anchor AND xpath LIKE CONCAT('%/',:descendantName)",\r
+        nativeQuery = true)\r
+    // Above query will match the anchor id and last descendant name\r
+    List<FragmentEntity> getByAnchorAndXpathEndsInDescendantName(@Param("anchor") int anchorId,\r
+                                                                 @Param("descendantName") String descendantName);\r
+}\r

diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy
index 231a572..a2c7a09 100644 (file)
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy
@@ -338,9 +338,9 @@ class CpsDataPersistenceServiceSpec extends CpsPersistenceSpecBase {
             dataNode.getLeaves().toString() == leaves
             dataNode.getChildDataNodes().size() == expectedNumberOfChidlNodes
         where: 'the following data is used'
-            type                        | cpsPath                                                          | includeDescendantsOption | expectedNumberOfChidlNodes
-            'String and no descendants' | '/parent-200/child-202[@common-leaf-name=\'common-leaf-value\']' | OMIT_DESCENDANTS         | 0
-            'Integer and descendants'   | '/parent-200/child-202[@common-leaf-name-int=5]'                 | INCLUDE_ALL_DESCENDANTS  | 1
+            type                        | cpsPath                                                          | includeDescendantsOption || expectedNumberOfChidlNodes
+            'String and no descendants' | '/parent-200/child-202[@common-leaf-name=\'common-leaf-value\']' | OMIT_DESCENDANTS         || 0
+            'Integer and descendants'   | '/parent-200/child-202[@common-leaf-name-int=5]'                 | INCLUDE_ALL_DESCENDANTS  || 1
     }
 
     @Unroll
@@ -355,35 +355,35 @@ class CpsDataPersistenceServiceSpec extends CpsPersistenceSpecBase {
             'cps path is incomplete'           | '/parent-200[@common-leaf-name-int=5]'
             'leaf value does not exist'        | '/parent-200/child-202[@common-leaf-name=\'does not exist\']'
             'incomplete end of xpath prefix'   | '/parent-200/child-20[@common-leaf-name-int=5]'
-            'empty cps path of type ends with' | '///'
     }
 
     @Unroll
     @Sql([CLEAR_DATA, SET_DATA])
-    def 'Cps Path query with and without descendants using #type.'() {
+    def 'Cps Path query using descendant anywhere and #type (further) descendants.'() {
         when: 'a query is executed to get a data node by the given cps path'
-            def cpsPath = '///child-202'
+            def cpsPath = '//child-202'
             def result = objectUnderTest.queryDataNodes(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, cpsPath, includeDescendantsOption)
         then: 'the data node has the correct number of children'
             DataNode dataNode = result.stream().findFirst().get()
             dataNode.getChildDataNodes().size() == expectedNumberOfChildNodes
         where: 'the following data is used'
-            type                                    | includeDescendantsOption | expectedNumberOfChildNodes
-            'ends with and omit descendants'        | OMIT_DESCENDANTS         | 0
-            'ends with and include all descendants' | INCLUDE_ALL_DESCENDANTS  | 1
+            type      | includeDescendantsOption || expectedNumberOfChildNodes
+            'omit'    | OMIT_DESCENDANTS         || 0
+            'include' | INCLUDE_ALL_DESCENDANTS  || 1
     }
 
     @Unroll
     @Sql([CLEAR_DATA, SET_DATA])
-    def 'Cps Path query using ends with variations of #type.'() {
+    def 'Cps Path query using descendant anywhere with %scenario '() {
         when: 'a query is executed to get a data node by the given cps path'
             def result = objectUnderTest.queryDataNodes(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, cpsPath, OMIT_DESCENDANTS)
-        then: 'the correct number of data nodes is returned'
-            result.size() == expectedNumberOfDataNodes
+        then: 'Only one data node is returned'
+            result.size() == 1
+        and:
+            result.stream().findFirst().get().xpath == expectedXPath
         where: 'the following data is used'
-            type                            | cpsPath             | expectedNumberOfDataNodes
-            'single match with / prefix'    | '///child-202'      | 1
-            'single match without / prefix' | '//grand-child-202' | 1
-            'multiple matches'              | '//202'             | 2
+            scenario                                  | cpsPath             || expectedXPath
+            'fully unique descendant name'            | '//grand-child-202' || '/parent-200/child-202/grand-child-202'
+            'descendant name match end of other node' | '//child-202'       || '/parent-200/child-202'
     }
 }

diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy
index 0bc99c6..7f558dd 100644 (file)
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy
@@ -50,13 +50,14 @@ class CpsPathQuerySpec extends Specification {
         when: 'the given cps path is parsed'
             def result = objectUnderTest.createFrom(cpsPath)
         then: 'the query has the right type'
-            result.cpsPathQueryType == CpsPathQueryType.XPATH_ENDS_WITH
+            result.cpsPathQueryType == CpsPathQueryType.XPATH_HAS_DESCENDANT_ANYWHERE
         and: 'the right ends with parameters are set'
-            result.endsWith == expectedEndsWithValue
+            result.descendantName == expectedEndsWithValue
         where: 'the following data is used'
-            scenario         | cpsPath                   || expectedEndsWithValue
-            'yang container' | '///cps-path'             || '/cps-path'
-            'yang list'      | '///cps-path[@key=value]' || '/cps-path[@key=value]'
+            scenario         | cpsPath                  || expectedEndsWithValue
+            'yang container' | '//cps-path'             || 'cps-path'
+            'yang list'      | '//cps-path[@key=value]' || 'cps-path[@key=value]'
+            'parent & child' | '//parent/child'         || 'parent/child'
     }
 
     @Unroll
@@ -66,9 +67,10 @@ class CpsPathQuerySpec extends Specification {
         then: 'a CpsPathException is thrown'
             thrown(CpsPathException)
         where: 'the following data is used'
-            scenario              | cpsPath
-            'no / at the start'   | 'invalid-cps-path/child'
-            'float value'         | '/parent-200/child-202[@common-leaf-name-float=5.0]'
-            'too many containers' | '/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17/18/19/20/21/22/23/24/25/26/27/28/29/30/31/32/33/34/35/36/37/38/39/40/41/42/43/44/45/46/47/48/49/50/51/52/53/54/55/56/57/58/59/60/61/62/63/64/65/66/67/68/69/70/71/72/73/74/75/76/77/78/79/80/81/82/83/84/85/86/87/88/89/90/91/92/93/94/95/96/97/98/99/100[@a=1]'
+            scenario                               | cpsPath
+            'no / at the start'                    | 'invalid-cps-path/child'
+            'additional / after descendant option' | '///cps-path'
+            'float value'                          | '/parent-200/child-202[@common-leaf-name-float=5.0]'
+            'too many containers'                  | '/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17/18/19/20/21/22/23/24/25/26/27/28/29/30/31/32/33/34/35/36/37/38/39/40/41/42/43/44/45/46/47/48/49/50/51/52/53/54/55/56/57/58/59/60/61/62/63/64/65/66/67/68/69/70/71/72/73/74/75/76/77/78/79/80/81/82/83/84/85/86/87/88/89/90/91/92/93/94/95/96/97/98/99/100[@a=1]'
     }
 }