Fix Delete uses case with '/' in path

-Extend and use cpsPath parser (util) classes instead of regex

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

7 files changed, 142 insertions, 18 deletions

diff --git a/cps-path-parser/src/main/antlr4/org/onap/cps/cpspath/parser/antlr4/CpsPath.g4 b/cps-path-parser/src/main/antlr4/org/onap/cps/cpspath/parser/antlr4/CpsPath.g4
index 40ad410a0d..db09b3c532 100644
--- a/cps-path-parser/src/main/antlr4/org/onap/cps/cpspath/parser/antlr4/CpsPath.g4
+++ b/cps-path-parser/src/main/antlr4/org/onap/cps/cpspath/parser/antlr4/CpsPath.g4
@@ -28,7 +28,9 @@ ancestorPath : yangElement ( SLASH yangElement)* ;
 
 textFunctionCondition : SLASH leafName OB KW_TEXT_FUNCTION EQ StringLiteral CB ;
 
-prefix : ( SLASH yangElement)* SLASH containerName ;
+parent : ( SLASH yangElement)* ;
+
+prefix : parent SLASH containerName ;
 
 descendant : SLASH prefix ;
 
diff --git a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java
index 21f5173a98..7183120120 100644
--- a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java
+++ b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java
@@ -61,6 +61,11 @@ public class CpsPathBuilder extends CpsPathBaseListener {
     }
 
     @Override
+    public void exitParent(final CpsPathParser.ParentContext ctx) {
+        cpsPathQuery.setNormalizedParentPath(normalizedXpathBuilder.toString());
+    }
+
+    @Override
     public void exitIncorrectPrefix(final IncorrectPrefixContext ctx) {
         throw new PathParsingException("CPS path can only start with one or two slashes (/)");
     }
diff --git a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathQuery.java b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathQuery.java
index 53490f3a2d..a9bd5d81c3 100644
--- a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathQuery.java
+++ b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathQuery.java
@@ -32,6 +32,7 @@ import lombok.Setter;
 public class CpsPathQuery {
 
     private String xpathPrefix;
+    private String normalizedParentPath;
     private String normalizedXpath;
     private CpsPathPrefixType cpsPathPrefixType = ABSOLUTE;
     private String descendantName;
diff --git a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathUtil.java b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathUtil.java
index 97d7d1d760..283463b512 100644
--- a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathUtil.java
+++ b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathUtil.java
@@ -20,6 +20,8 @@
 
 package org.onap.cps.cpspath.parser;
 
+import static org.onap.cps.cpspath.parser.CpsPathPrefixType.ABSOLUTE;
+
 import lombok.AccessLevel;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
@@ -45,8 +47,29 @@ public class CpsPathUtil {
      * @return a normalized xpath String.
      */
     public static String getNormalizedXpath(final String xpathSource) {
-        final CpsPathBuilder cpsPathBuilder = getCpsPathBuilder(xpathSource);
-        return cpsPathBuilder.build().getNormalizedXpath();
+        return getCpsPathBuilder(xpathSource).build().getNormalizedXpath();
+    }
+
+    /**
+     * Returns the parent xpath.
+     *
+     * @param xpathSource xpath
+     * @return the parent xpath String.
+     */
+    public static String getNormalizedParentXpath(final String xpathSource) {
+        return getCpsPathBuilder(xpathSource).build().getNormalizedParentPath();
+    }
+
+
+    /**
+     * Returns boolean indicating xpath is an absolute path to a list element.
+     *
+     * @param xpathSource xpath
+     * @return true if xpath is an absolute path to a list element
+     */
+    public static boolean isPathToListElement(final String xpathSource) {
+        final CpsPathQuery cpsPathQuery = getCpsPathBuilder(xpathSource).build();
+        return cpsPathQuery.getCpsPathPrefixType() == ABSOLUTE && cpsPathQuery.hasLeafConditions();
     }
 
     /**
@@ -57,8 +80,7 @@ public class CpsPathUtil {
      */
 
     public static CpsPathQuery getCpsPathQuery(final String cpsPathSource) {
-        final CpsPathBuilder cpsPathBuilder = getCpsPathBuilder(cpsPathSource);
-        return cpsPathBuilder.build();
+        return getCpsPathBuilder(cpsPathSource).build();
     }
 
     private static CpsPathBuilder getCpsPathBuilder(final String cpsPathSource) {
diff --git a/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathUtilSpec.groovy b/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathUtilSpec.groovy
new file mode 100644
index 0000000000..31c1ed4a3a
--- /dev/null
+++ b/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathUtilSpec.groovy
@@ -0,0 +1,74 @@
+/*
+ *  ============LICENSE_START=======================================================
+ *  Copyright (C) 2022 Nordix Foundation
+ *  ================================================================================
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ *  SPDX-License-Identifier: Apache-2.0
+ *  ============LICENSE_END=========================================================
+ */
+
+package org.onap.cps.cpspath.parser
+
+import spock.lang.Specification
+
+class CpsPathUtilSpec extends Specification {
+
+    def 'Normalized xpaths for list index values using #scenario'() {
+        when: 'xpath with #scenario is parsed'
+            def result = CpsPathUtil.getNormalizedXpath(xpath)
+        then: 'normalized path uses single quotes for leave values'
+            result == "/parent/child[@common-leaf-name='123']"
+        where: 'the following xpaths are used'
+            scenario        | xpath
+            'no quotes'     | '/parent/child[@common-leaf-name=123]'
+            'double quotes' | '/parent/child[@common-leaf-name="123"]'
+            'single quotes' | "/parent/child[@common-leaf-name='123']"
+    }
+
+    def 'Normalized parent xpaths'() {
+        when: 'a given xpath with #scenario is parsed'
+            def result = CpsPathUtil.getNormalizedParentXpath(xpath)
+        then: 'the result is the expected parent path'
+            result == expectedParentPath
+        where: 'the following xpaths are used'
+            scenario                         | xpath                                 || expectedParentPath
+            'no child'                       | '/parent'                             || ''
+            'child and parent'               | '/parent/child'                       || '/parent'
+            'grand child'                    | '/parent/child/grandChild'            || '/parent/child'
+            'parent & top is list element'   | '/parent[@id=1]/child'                || "/parent[@id='1']"
+            'parent is list element'         | '/parent/child[@id=1]/grandChild'     || "/parent/child[@id='1']"
+            'parent is list element with /'  | "/parent/child[@id='a/b']/grandChild" || "/parent/child[@id='a/b']"
+            'parent is list element with ['  | "/parent/child[@id='a[b']/grandChild" || "/parent/child[@id='a[b']"
+            'parent is list element using "' | '/parent/child[@id="x"]/grandChild'   || "/parent/child[@id='x']"
+    }
+
+    def 'Recognizing (absolute) xpaths to List elements'() {
+        expect: 'check for list returns the correct values'
+            assert CpsPathUtil.isPathToListElement(xpath) == expectList
+        where: 'the following xpaths are used'
+            xpath                  || expectList
+            '/parent[@id=1]'       || true
+            '/parent[@id=1]/child' || false
+            '/parent/child[@id=1]' || true
+            '//child[@id=1]'       || false
+    }
+
+    def 'Parsing Exception'() {
+        when: 'a invalid xpath is parsed'
+            CpsPathUtil.getNormalizedXpath('///')
+        then: 'a path parsing exception is thrown'
+            thrown(PathParsingException)
+    }
+
+}
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 b7da66e467..82bcea2f1a 100644
--- 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
@@ -79,9 +79,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
     private final SessionManager sessionManager;
 
     private static final String REG_EX_FOR_OPTIONAL_LIST_INDEX = "(\\[@[\\s\\S]+?]){0,1})";
-    private static final Pattern REG_EX_PATTERN_FOR_LIST_ELEMENT_KEY_PREDICATE =
-            Pattern.compile("\\[(\\@([^\\/]{0,9999}))\\]$");
-    private static final String TOP_LEVEL_MODULE_PREFIX_PROPERTY_NAME = "topLevelModulePrefix";
 
     @Override
     public void addChildDataNode(final String dataspaceName, final String anchorName, final String parentNodeXpath,
@@ -550,13 +547,10 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
             if (isRootContainerNodeXpath(targetXpath)) {
                 parentNodeXpath = targetXpath;
             } else {
-                parentNodeXpath = targetXpath.substring(0, targetXpath.lastIndexOf('/'));
+                parentNodeXpath = CpsPathUtil.getNormalizedParentXpath(targetXpath);
             }
             parentFragmentEntity = getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath);
-            final String lastXpathElement = targetXpath.substring(targetXpath.lastIndexOf('/'));
-            final boolean isListElement = REG_EX_PATTERN_FOR_LIST_ELEMENT_KEY_PREDICATE
-                    .matcher(lastXpathElement).find();
-            if (isListElement) {
+            if (CpsPathUtil.isPathToListElement(targetXpath)) {
                 targetDeleted = deleteDataNode(parentFragmentEntity, targetXpath);
             } else {
                 targetDeleted = deleteAllListElements(parentFragmentEntity, targetXpath);
diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy
index 12585eb5a9..cc2369d50e 100755
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy
@@ -38,6 +38,7 @@ import org.onap.cps.spi.model.DataNodeBuilder
 import org.onap.cps.utils.JsonObjectMapper
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.test.context.jdbc.Sql
+
 import javax.validation.ConstraintViolationException
 
 import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS
@@ -68,6 +69,10 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
     static Collection<DataNode> existingDataNodes = [createDataNodeTree(XPATH_DATA_NODE_WITH_DESCENDANTS)]
     static Collection<DataNode> existingChildDataNodes = [createDataNodeTree('/parent-1/child-1')]
 
+    def static deleteTestParentXPath = '/parent-200'
+    def static deleteTestChildXpath = "${deleteTestParentXPath}/child-with-slash[@key='a/b']"
+    def static deleteTestGrandChildXPath = "${deleteTestChildXpath}/grandChild"
+
     def expectedLeavesByXpathMap = [
             '/parent-207'                      : ['parent-leaf': 'parent-leaf value'],
             '/parent-207/child-001'            : ['first-child-leaf': 'first-child-leaf value'],
@@ -286,7 +291,8 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
             scenario                 | dataspaceName  | anchorName                        | xpath           || expectedException
             'non-existing dataspace' | 'NO DATASPACE' | 'not relevant'                    | '/not relevant' || DataspaceNotFoundException
             'non-existing anchor'    | DATASPACE_NAME | 'NO ANCHOR'                       | '/not relevant' || AnchorNotFoundException
-            'non-existing xpath'     | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | '/NO XPATH'     || DataNodeNotFoundException
+            'non-existing xpath'     | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | '/NO-XPATH'     || DataNodeNotFoundException
+            'invalid xpath'          | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | 'INVALID XPATH' || CpsPathException
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
@@ -316,7 +322,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
             scenario                 | dataspaceName  | anchorName                        | xpath                 || expectedException
             'non-existing dataspace' | 'NO DATASPACE' | 'not relevant'                    | '/not relevant'       || DataspaceNotFoundException
             'non-existing anchor'    | DATASPACE_NAME | 'NO ANCHOR'                       | '/not relevant'       || AnchorNotFoundException
-            'non-existing xpath'     | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | '/NON-EXISTING XPATH' || DataNodeNotFoundException
+            'non-existing xpath'     | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | '/NON-EXISTING-XPATH' || DataNodeNotFoundException
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
@@ -410,7 +416,8 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
             scenario                 | dataspaceName  | anchorName                        | xpath                 || expectedException
             'non-existing dataspace' | 'NO DATASPACE' | 'not relevant'                    | '/not relevant'       || DataspaceNotFoundException
             'non-existing anchor'    | DATASPACE_NAME | 'NO ANCHOR'                       | '/not relevant'       || AnchorNotFoundException
-            'non-existing xpath'     | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | '/NON-EXISTING XPATH' || DataNodeNotFoundException
+            'non-existing xpath'     | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | '/NON-EXISTING-XPATH' || DataNodeNotFoundException
+            'invalid xpath'          | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | 'INVALID XPATH'       || CpsPathException
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
@@ -523,6 +530,25 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
+    def 'Delete data nodes with "/"-token in list key value: #scenario. (CPS-1409)'() {
+        given: 'a data nodes with list-element child with "/" in index value (and grandchild)'
+            def grandChild = new DataNodeBuilder().withXpath(deleteTestGrandChildXPath).build()
+            def child = new DataNodeBuilder().withXpath(deleteTestChildXpath).withChildDataNodes([grandChild]).build()
+            objectUnderTest.addChildDataNode(DATASPACE_NAME, ANCHOR_NAME3, deleteTestParentXPath, child)
+        and: 'number of children before delete is stored'
+            def numberOfChildrenBeforeDelete = objectUnderTest.getDataNode(DATASPACE_NAME, ANCHOR_NAME3, pathToParentOfDeletedNode, INCLUDE_ALL_DESCENDANTS).childDataNodes.size()
+        when: 'target node is deleted'
+            objectUnderTest.deleteDataNode(DATASPACE_NAME, ANCHOR_NAME3, deleteTarget)
+        then: 'one child has been deleted'
+            def numberOfChildrenAfterDelete = objectUnderTest.getDataNode(DATASPACE_NAME, ANCHOR_NAME3, pathToParentOfDeletedNode, INCLUDE_ALL_DESCENDANTS).childDataNodes.size()
+            assert numberOfChildrenAfterDelete == numberOfChildrenBeforeDelete - 1
+        where:
+            scenario                | deleteTarget              | pathToParentOfDeletedNode
+            'list element with /'   | deleteTestChildXpath      | deleteTestParentXPath
+            'child of list element' | deleteTestGrandChildXPath | deleteTestChildXpath
+    }
+
+    @Sql([CLEAR_DATA, SET_DATA])
     def 'Delete list error scenario: #scenario.'() {
         when: 'attempting to delete scenario: #scenario.'
             objectUnderTest.deleteListDataNode(DATASPACE_NAME, ANCHOR_NAME3, targetXpaths)
@@ -539,7 +565,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
-    def 'Confirm deletion of #scenario.'() {
+    def 'Delete data node by xpath #scenario.'() {
         given: 'a valid data node'
             def dataNode
         and: 'data nodes are deleted'
@@ -564,7 +590,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
-    def 'Delete data node with #scenario.'() {
+    def 'Delete data node error scenario: #scenario.'() {
         when: 'data node is deleted'
             objectUnderTest.deleteDataNode(DATASPACE_NAME, ANCHOR_NAME3, datanodeXpath)
         then: 'a #expectedException is thrown'