diff options
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 40ad410a0..db09b3c53 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 21f5173a9..718312012 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 53490f3a2..a9bd5d81c 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 97d7d1d76..283463b51 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 000000000..31c1ed4a3 --- /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 b7da66e46..82bcea2f1 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 12585eb5a..cc2369d50 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' |