From 5452b5ea786a3e7def19c1f328c43ec264da38d5 Mon Sep 17 00:00:00 2001 From: emaclee Date: Wed, 21 Dec 2022 09:29:24 +0000 Subject: Add fix for posting nodes with xPath with '/' - YangUtils method changed from using REGEX to cps path parser - unit test added for cps path util Issue-ID: CPS-1433 Signed-off-by: emaclee Change-Id: Ibb9efdd09423f9bade4a4a557d7d9ed49aa44ef4 --- .../org/onap/cps/cpspath/parser/CpsPathBuilder.java | 11 +++++++++-- .../org/onap/cps/cpspath/parser/CpsPathQuery.java | 2 ++ .../org/onap/cps/cpspath/parser/CpsPathUtil.java | 6 ++++++ .../onap/cps/cpspath/parser/CpsPathUtilSpec.groovy | 20 ++++++++++++++++++-- cps-service/pom.xml | 4 ++++ .../src/main/java/org/onap/cps/utils/YangUtils.java | 19 +++++++------------ .../org/onap/cps/utils/JsonObjectMapperSpec.groovy | 2 +- cps-service/src/test/resources/bookstore.json | 8 ++++---- csit/data/test-tree.json | 6 +++--- csit/tests/cps-data/cps-data.robot | 4 ++-- docs/release-notes.rst | 2 ++ 11 files changed, 58 insertions(+), 26 deletions(-) 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 7183120120..3a9d70ebbc 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 @@ -22,7 +22,9 @@ package org.onap.cps.cpspath.parser; import static org.onap.cps.cpspath.parser.CpsPathPrefixType.DESCENDANT; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.onap.cps.cpspath.parser.antlr4.CpsPathBaseListener; import org.onap.cps.cpspath.parser.antlr4.CpsPathParser; @@ -50,6 +52,8 @@ public class CpsPathBuilder extends CpsPathBaseListener { boolean processingAncestorAxis = false; + private List containerNames = new ArrayList<>(); + @Override public void exitInvalidPostFix(final CpsPathParser.InvalidPostFixContext ctx) { throw new PathParsingException(ctx.getText()); @@ -146,6 +150,7 @@ public class CpsPathBuilder extends CpsPathBaseListener { CpsPathQuery build() { cpsPathQuery.setNormalizedXpath(normalizedXpathBuilder.toString()); + cpsPathQuery.setContainerNames(containerNames); return cpsPathQuery; } @@ -155,10 +160,12 @@ public class CpsPathBuilder extends CpsPathBaseListener { @Override public void exitContainerName(final CpsPathParser.ContainerNameContext ctx) { + final String containerName = ctx.getText(); normalizedXpathBuilder.append("/") - .append(ctx.getText()); + .append(containerName); + containerNames.add(containerName); if (processingAncestorAxis) { - normalizedAncestorPathBuilder.append("/").append(ctx.getText()); + normalizedAncestorPathBuilder.append("/").append(containerName); } } 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 a9bd5d81c3..c9df8df904 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 @@ -22,6 +22,7 @@ package org.onap.cps.cpspath.parser; import static org.onap.cps.cpspath.parser.CpsPathPrefixType.ABSOLUTE; +import java.util.List; import java.util.Map; import lombok.AccessLevel; import lombok.Getter; @@ -34,6 +35,7 @@ public class CpsPathQuery { private String xpathPrefix; private String normalizedParentPath; private String normalizedXpath; + private List containerNames; private CpsPathPrefixType cpsPathPrefixType = ABSOLUTE; private String descendantName; private Map leavesData; 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 283463b512..60f0e2efcd 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 @@ -22,6 +22,7 @@ package org.onap.cps.cpspath.parser; import static org.onap.cps.cpspath.parser.CpsPathPrefixType.ABSOLUTE; +import java.util.List; import lombok.AccessLevel; import lombok.Getter; import lombok.NoArgsConstructor; @@ -60,6 +61,11 @@ public class CpsPathUtil { return getCpsPathBuilder(xpathSource).build().getNormalizedParentPath(); } + public static String[] getXpathNodeIdSequence(final String xpathSource) { + final List containerNames = getCpsPathBuilder(xpathSource).build().getContainerNames(); + return containerNames.toArray(new String[containerNames.size()]); + } + /** * Returns boolean indicating xpath is an absolute path to a list element. 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 index 31c1ed4a3a..d62f337b75 100644 --- 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 @@ -28,7 +28,7 @@ class CpsPathUtilSpec extends Specification { 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']" + assert result == "/parent/child[@common-leaf-name='123']" where: 'the following xpaths are used' scenario | xpath 'no quotes' | '/parent/child[@common-leaf-name=123]' @@ -40,7 +40,7 @@ class CpsPathUtilSpec extends Specification { when: 'a given xpath with #scenario is parsed' def result = CpsPathUtil.getNormalizedParentXpath(xpath) then: 'the result is the expected parent path' - result == expectedParentPath + assert result == expectedParentPath where: 'the following xpaths are used' scenario | xpath || expectedParentPath 'no child' | '/parent' || '' @@ -53,6 +53,22 @@ class CpsPathUtilSpec extends Specification { 'parent is list element using "' | '/parent/child[@id="x"]/grandChild' || "/parent/child[@id='x']" } + def 'Get node ID sequence for given xpath'() { + when: 'a given xpath with #scenario is parsed' + def result = CpsPathUtil.getXpathNodeIdSequence(xpath) + then: 'the result is the expected node ID sequence' + assert result == expectedNodeIdSequence + where: 'the following xpaths are used' + scenario | xpath || expectedNodeIdSequence + 'no child' | '/parent' || ["parent"] + 'child and parent' | '/parent/child' || ["parent","child"] + 'grand child' | '/parent/child/grandChild' || ["parent","child","grandChild"] + 'parent & top is list element' | '/parent[@id=1]/child' || ["parent","child"] + 'parent is list element' | '/parent/child[@id=1]/grandChild' || ["parent","child","grandChild"] + 'parent is list element with /' | "/parent/child[@id='a/b']/grandChild" || ["parent","child","grandChild"] + 'parent is list element with [' | "/parent/child[@id='a[b']/grandChild" || ["parent","child","grandChild"] + } + def 'Recognizing (absolute) xpaths to List elements'() { expect: 'check for list returns the correct values' assert CpsPathUtil.isPathToListElement(xpath) == expectList diff --git a/cps-service/pom.xml b/cps-service/pom.xml index 77f262c32e..0cefb8fcc2 100644 --- a/cps-service/pom.xml +++ b/cps-service/pom.xml @@ -110,6 +110,10 @@ org.codehaus.janino janino + + org.onap.cps + cps-path-parser + com.hazelcast diff --git a/cps-service/src/main/java/org/onap/cps/utils/YangUtils.java b/cps-service/src/main/java/org/onap/cps/utils/YangUtils.java index 9a61579b12..3ef6c6fcf7 100644 --- a/cps-service/src/main/java/org/onap/cps/utils/YangUtils.java +++ b/cps-service/src/main/java/org/onap/cps/utils/YangUtils.java @@ -28,7 +28,6 @@ import com.google.gson.stream.JsonReader; import java.io.IOException; import java.io.StringReader; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -37,6 +36,8 @@ import java.util.stream.Collectors; import lombok.AccessLevel; import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.onap.cps.cpspath.parser.CpsPathUtil; +import org.onap.cps.cpspath.parser.PathParsingException; import org.onap.cps.spi.exceptions.DataValidationException; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; @@ -60,9 +61,6 @@ import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack; @NoArgsConstructor(access = AccessLevel.PRIVATE) public class YangUtils { - private static final String XPATH_DELIMITER_REGEX = "\\/"; - private static final String XPATH_NODE_KEY_ATTRIBUTES_REGEX = "\\[.*?\\]"; - /** * Parses jsonData into Collection of NormalizedNode according to given schema context. * @@ -170,15 +168,12 @@ public class YangUtils { } private static String[] xpathToNodeIdSequence(final String xpath) { - final String[] xpathNodeIdSequence = Arrays.stream(xpath - .replaceAll(XPATH_NODE_KEY_ATTRIBUTES_REGEX, "") - .split(XPATH_DELIMITER_REGEX)) - .filter(identifier -> !identifier.isEmpty()) - .toArray(String[]::new); - if (xpathNodeIdSequence.length < 1) { - throw new DataValidationException("Invalid xpath.", "Xpath contains no node identifiers."); + try { + return CpsPathUtil.getXpathNodeIdSequence(xpath); + } catch (final PathParsingException pathParsingException) { + throw new DataValidationException(pathParsingException.getMessage(), pathParsingException.getDetails(), + pathParsingException); } - return xpathNodeIdSequence; } private static Collection findDataSchemaNodeIdentifiersByXpathNodeIdSequence( diff --git a/cps-service/src/test/groovy/org/onap/cps/utils/JsonObjectMapperSpec.groovy b/cps-service/src/test/groovy/org/onap/cps/utils/JsonObjectMapperSpec.groovy index e205a19eed..b70c437953 100644 --- a/cps-service/src/test/groovy/org/onap/cps/utils/JsonObjectMapperSpec.groovy +++ b/cps-service/src/test/groovy/org/onap/cps/utils/JsonObjectMapperSpec.groovy @@ -41,7 +41,7 @@ class JsonObjectMapperSpec extends Specification { then: 'the result is a valid json string (can be parsed)' def contentMap = new JsonSlurper().parseText(content) and: 'the parsed content is as expected' - assert contentMap.'test:bookstore'.'bookstore-name' == 'Chapters' + assert contentMap.'test:bookstore'.'bookstore-name' == 'Chapters/Easons' } def 'Map a structured object to json String error.'() { diff --git a/cps-service/src/test/resources/bookstore.json b/cps-service/src/test/resources/bookstore.json index d1b8d6882d..459908bd63 100644 --- a/cps-service/src/test/resources/bookstore.json +++ b/cps-service/src/test/resources/bookstore.json @@ -1,19 +1,19 @@ { "test:bookstore":{ - "bookstore-name": "Chapters", + "bookstore-name": "Chapters/Easons", "categories": [ { - "code": "01", + "code": "01/1", "name": "SciFi", "books": [ { "authors": [ "Iain M. Banks" ], - "lang": "en", + "lang": "en/it", "price": "895", "pub_year": "1994", - "title": "Feersum Endjinn" + "title": "Feersum Endjinn/Endjinn Feersum" }, { "authors": [ diff --git a/csit/data/test-tree.json b/csit/data/test-tree.json index 89d6784275..8f4b522799 100644 --- a/csit/data/test-tree.json +++ b/csit/data/test-tree.json @@ -2,11 +2,11 @@ "test-tree": { "branch": [ { - "name": "Left", + "name": "LEFT/left", "nest": { - "name": "Small", + "name": "SMALL/small", "birds": [ - "Sparrow", + "SPARROW/sparrow", "Robin", "Finch" ] diff --git a/csit/tests/cps-data/cps-data.robot b/csit/tests/cps-data/cps-data.robot index 2da2b73414..096bd07b79 100644 --- a/csit/tests/cps-data/cps-data.robot +++ b/csit/tests/cps-data/cps-data.robot @@ -44,10 +44,10 @@ Create Data Node Get Data Node by XPath ${uri}= Set Variable ${basePath}/v1/dataspaces/${dataspaceName}/anchors/${anchorName}/node - ${params}= Create Dictionary xpath=/test-tree/branch[@name='Left']/nest + ${params}= Create Dictionary xpath=/test-tree/branch[@name='LEFT/left']/nest ${headers}= Create Dictionary Authorization=${auth} ${response}= Get On Session CPS_URL ${uri} params=${params} headers=${headers} expected_status=200 ${responseJson}= Set Variable ${response.json()['tree:nest']} - Should Be Equal As Strings ${responseJson['name']} Small + Should Be Equal As Strings ${responseJson['name']} SMALL/small diff --git a/docs/release-notes.rst b/docs/release-notes.rst index 32219000b5..9aaf0501d5 100755 --- a/docs/release-notes.rst +++ b/docs/release-notes.rst @@ -50,6 +50,8 @@ Bug Fixes 3.2.0 - `CPS-1312 `_ CPS(/NCMP) does not have version control - `CPS-1350 `_ [CPS/NCMP] Add Basic Auth to CPS/NCMP OpenAPI Definitions + - `CPS-1433 `_ [CPS/NCMP] Fix to allow posting data with '/' + - `CPS-1409 `_ [CPS/NCMP] Fix Delete uses case with '/' in path Known Limitations, Issues and Workarounds ----------------------------------------- -- cgit 1.2.3-korg