diff options
5 files changed, 48 insertions, 49 deletions
diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java index 34dea9bc19..7b5c0c693f 100644 --- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java @@ -44,9 +44,6 @@ import org.springframework.stereotype.Component; @Slf4j @Component public class FragmentQueryBuilder { - private static final String REGEX_ABSOLUTE_PATH_PREFIX = "^"; - private static final String REGEX_DESCENDANT_PATH_PREFIX = "^.*\\/"; - private static final String REGEX_OPTIONAL_LIST_INDEX_POSTFIX = "(\\[@(?!.*\\[).*?])?$"; private static final AnchorEntity ACROSS_ALL_ANCHORS = null; @PersistenceContext @@ -77,12 +74,6 @@ public class FragmentQueryBuilder { return getQueryForDataspaceOrAnchorAndCpsPath(dataspaceEntity, ACROSS_ALL_ANCHORS, cpsPathQuery); } - private static String getXpathSqlRegex(final CpsPathQuery cpsPathQuery) { - final StringBuilder xpathRegexBuilder = getRegexStringBuilderWithPrefix(cpsPathQuery); - xpathRegexBuilder.append(REGEX_OPTIONAL_LIST_INDEX_POSTFIX); - return xpathRegexBuilder.toString(); - } - private Query getQueryForDataspaceOrAnchorAndCpsPath(final DataspaceEntity dataspaceEntity, final AnchorEntity anchorEntity, final CpsPathQuery cpsPathQuery) { @@ -110,26 +101,13 @@ public class FragmentQueryBuilder { private static void addXpathSearch(final CpsPathQuery cpsPathQuery, final StringBuilder sqlStringBuilder, final Map<String, Object> queryParameters) { - sqlStringBuilder.append(" AND xpath ~ :xpathRegex"); - final String xpathRegex = getXpathSqlRegex(cpsPathQuery); - queryParameters.put("xpathRegex", xpathRegex); - } - - private static StringBuilder getRegexStringBuilderWithPrefix(final CpsPathQuery cpsPathQuery) { - final StringBuilder xpathRegexBuilder = new StringBuilder(); + sqlStringBuilder.append(" AND (xpath LIKE :escapedXpath OR " + + "(xpath LIKE :escapedXpath||'[@%]' AND xpath NOT LIKE :escapedXpath||'[@%]/%[@%]'))"); if (CpsPathPrefixType.ABSOLUTE.equals(cpsPathQuery.getCpsPathPrefixType())) { - xpathRegexBuilder.append(REGEX_ABSOLUTE_PATH_PREFIX); - xpathRegexBuilder.append(escapeXpath(cpsPathQuery.getXpathPrefix())); - return xpathRegexBuilder; + queryParameters.put("escapedXpath", EscapeUtils.escapeForSqlLike(cpsPathQuery.getXpathPrefix())); + } else { + queryParameters.put("escapedXpath", "%/" + EscapeUtils.escapeForSqlLike(cpsPathQuery.getDescendantName())); } - xpathRegexBuilder.append(REGEX_DESCENDANT_PATH_PREFIX); - xpathRegexBuilder.append(escapeXpath(cpsPathQuery.getDescendantName())); - return xpathRegexBuilder; - } - - private static String escapeXpath(final String xpath) { - // See https://jira.onap.org/browse/CPS-500 for limitations of this basic escape mechanism - return xpath.replace("[@", "\\[@"); } private static Integer getTextValueAsInt(final CpsPathQuery cpsPathQuery) { diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/utils/EscapeUtilsSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/utils/EscapeUtilsSpec.groovy index 17eb8846a1..7de9b97ba0 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/utils/EscapeUtilsSpec.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/utils/EscapeUtilsSpec.groovy @@ -25,8 +25,8 @@ import spock.lang.Specification class EscapeUtilsSpec extends Specification { def 'Escape text for using in SQL LIKE operation'() { - expect: - EscapeUtils.escapeForSqlLike(unescapedText) == escapedText + expect: 'SQL LIKE special characters to be escaped with forward-slash' + assert EscapeUtils.escapeForSqlLike(unescapedText) == escapedText where: unescapedText || escapedText 'Only %, _, and \\ are special' || 'Only \\%, \\_, and \\\\ are special' diff --git a/docs/cps-path.rst b/docs/cps-path.rst index bb482c2ed0..796eb7f429 100644 --- a/docs/cps-path.rst +++ b/docs/cps-path.rst @@ -1,6 +1,6 @@ .. This work is licensed under a Creative Commons Attribution 4.0 International License. .. http://creativecommons.org/licenses/by/4.0 -.. Copyright (C) 2021-2022 Nordix Foundation +.. Copyright (C) 2021-2023 Nordix Foundation .. Modifications Copyright (C) 2023 TechMahindra Ltd .. DO NOT CHANGE THIS LABEL FOR RELEASE NOTES - EVEN THOUGH IT GIVES A WARNING @@ -178,7 +178,6 @@ General Notes - String values must be wrapped in quotation marks ``"`` (U+0022) or apostrophes ``'`` (U+0027). - String comparisons are case sensitive. -- List key-fields containing ``\`` or ``@[`` will not be processed correctly when being referenced with such key values in absolute or descendant paths. This means such entries will be omitted from any query result. See `CPS-500 <https://jira.onap.org/browse/CPS-500>`_ Special Character Limitations of cpsPath Queries Query Syntax ============ diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy index 0cb3200f80..a736ab0c0e 100644 --- a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy +++ b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy @@ -21,6 +21,7 @@ package org.onap.cps.integration.functional +import java.time.OffsetDateTime import org.onap.cps.api.CpsQueryService import org.onap.cps.integration.base.FunctionalSpecBase import org.onap.cps.spi.FetchDescendantsOption @@ -339,15 +340,36 @@ class CpsQueryServiceIntegrationSpec extends FunctionalSpecBase { 'incomplete absolute 1 list entry' | '/categories[@code="3"]' || 0 } - def 'Cps Path query should ignore special characters: #scenario.'() { - when: 'a query is executed to get data nodes by the given cps path' + def 'Cps Path query contains #wildcard.'() { + when: 'a query is executed with a wildcard in the given cps path' def result = objectUnderTest.queryDataNodes(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, cpsPath, INCLUDE_ALL_DESCENDANTS) - then: 'no data nodes are returned' + then: 'no results are returned, as Cps Path query does not interpret wildcard characters' assert result.isEmpty() where: - scenario | cpsPath + wildcard | cpsPath + ' sql wildcard in parent path list index' | '/bookstore/categories[@code="%"]/books' + 'regex wildcard in parent path list index' | '/bookstore/categories[@code=".*"]/books' + ' sql wildcard in leaf-condition' | '/bookstore/categories[@code="1"]/books[@title="%"]' + 'regex wildcard in leaf-condition' | '/bookstore/categories[@code="1"]/books[@title=".*"]' + ' sql wildcard in text-condition' | '/bookstore/categories[@code="1"]/books/title[text()="%"]' + 'regex wildcard in text-condition' | '/bookstore/categories[@code="1"]/books/title[text()=".*"]' ' sql wildcard in contains-condition' | '/bookstore/categories[@code="1"]/books[contains(@title, "%")]' 'regex wildcard in contains-condition' | '/bookstore/categories[@code="1"]/books[contains(@title, ".*")]' } + def 'Cps Path query can return a data node containing [@ in xpath #scenario.'() { + given: 'a book with special characters [@ and ] in title' + cpsDataService.saveData(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, "/bookstore/categories[@code='1']", '{"books": [ {"title":"[@hello=world]"} ] }', OffsetDateTime.now()) + when: 'a query is executed' + def result = objectUnderTest.queryDataNodes(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, cpsPath, OMIT_DESCENDANTS) + then: 'the node is returned' + assert result.size() == 1 + cleanup: 'the new datanode' + cpsDataService.deleteDataNode(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, "/bookstore/categories[@code='1']/books[@title='[@hello=world]']", OffsetDateTime.now()) + where: + scenario || cpsPath + 'leaf-condition' || "/bookstore/categories[@code='1']/books[@title='[@hello=world]']" + 'text-condition' || "/bookstore/categories[@code='1']/books/title[text()='[@hello=world]']" + 'contains-condition' || "/bookstore/categories[@code='1']/books[contains(@title, '[@hello=world]')]" + } } diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/QueryPerfTest.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/QueryPerfTest.groovy index eafd16f346..bad3f8afd2 100644 --- a/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/QueryPerfTest.groovy +++ b/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/QueryPerfTest.groovy @@ -45,10 +45,10 @@ class QueryPerfTest extends CpsPerfTestBase { recordAndAssertPerformance("Query 1 anchor ${scenario}", durationLimit, durationInMillis) where: 'the following parameters are used' scenario | anchor | cpsPath || durationLimit | expectedNumberOfDataNodes - 'top element' | 'openroadm1' | '/openroadm-devices' || 200 | 50 * 86 + 1 - 'leaf condition' | 'openroadm2' | '//openroadm-device[@ne-state="inservice"]' || 250 | 50 * 86 - 'ancestors' | 'openroadm3' | '//openroadm-device/ancestor::openroadm-devices' || 200 | 50 * 86 + 1 - 'leaf condition + ancestors' | 'openroadm4' | '//openroadm-device[@status="success"]/ancestor::openroadm-devices' || 200 | 50 * 86 + 1 + 'top element' | 'openroadm1' | '/openroadm-devices' || 120 | 50 * 86 + 1 + 'leaf condition' | 'openroadm2' | '//openroadm-device[@ne-state="inservice"]' || 200 | 50 * 86 + 'ancestors' | 'openroadm3' | '//openroadm-device/ancestor::openroadm-devices' || 120 | 50 * 86 + 1 + 'leaf condition + ancestors' | 'openroadm4' | '//openroadm-device[@status="success"]/ancestor::openroadm-devices' || 120 | 50 * 86 + 1 } def 'Query complete data trees across all anchors with #scenario.'() { @@ -63,10 +63,10 @@ class QueryPerfTest extends CpsPerfTestBase { recordAndAssertPerformance("Query across anchors ${scenario}", durationLimit, durationInMillis) where: 'the following parameters are used' scenario | cpspath || durationLimit | expectedNumberOfDataNodes - 'top element' | '/openroadm-devices' || 600 | 5 * (50 * 86 + 1) - 'leaf condition' | '//openroadm-device[@ne-state="inservice"]' || 1000 | 5 * (50 * 86) - 'ancestors' | '//openroadm-device/ancestor::openroadm-devices' || 600 | 5 * (50 * 86 + 1) - 'leaf condition + ancestors' | '//openroadm-device[@status="success"]/ancestor::openroadm-devices' || 600 | 5 * (50 * 86 + 1) + 'top element' | '/openroadm-devices' || 400 | 5 * (50 * 86 + 1) + 'leaf condition' | '//openroadm-device[@ne-state="inservice"]' || 700 | 5 * (50 * 86) + 'ancestors' | '//openroadm-device/ancestor::openroadm-devices' || 400 | 5 * (50 * 86 + 1) + 'leaf condition + ancestors' | '//openroadm-device[@status="success"]/ancestor::openroadm-devices' || 400 | 5 * (50 * 86 + 1) } def 'Query with leaf condition and #scenario.'() { @@ -81,9 +81,9 @@ class QueryPerfTest extends CpsPerfTestBase { recordAndAssertPerformance("Query with ${scenario}", durationLimit, durationInMillis) where: 'the following parameters are used' scenario | fetchDescendantsOption | anchor || durationLimit | expectedNumberOfDataNodes - 'no descendants' | OMIT_DESCENDANTS | 'openroadm1' || 60 | 50 - 'direct descendants' | DIRECT_CHILDREN_ONLY | 'openroadm2' || 120 | 50 * 2 - 'all descendants' | INCLUDE_ALL_DESCENDANTS | 'openroadm3' || 200 | 50 * 86 + 'no descendants' | OMIT_DESCENDANTS | 'openroadm1' || 15 | 50 + 'direct descendants' | DIRECT_CHILDREN_ONLY | 'openroadm2' || 60 | 50 * 2 + 'all descendants' | INCLUDE_ALL_DESCENDANTS | 'openroadm3' || 150 | 50 * 86 } def 'Query ancestors with #scenario.'() { @@ -98,9 +98,9 @@ class QueryPerfTest extends CpsPerfTestBase { recordAndAssertPerformance("Query ancestors with ${scenario}", durationLimit, durationInMillis) where: 'the following parameters are used' scenario | fetchDescendantsOption | anchor || durationLimit | expectedNumberOfDataNodes - 'no descendants' | OMIT_DESCENDANTS | 'openroadm1' || 60 | 1 - 'direct descendants' | DIRECT_CHILDREN_ONLY | 'openroadm2' || 120 | 1 + 50 - 'all descendants' | INCLUDE_ALL_DESCENDANTS | 'openroadm3' || 200 | 1 + 50 * 86 + 'no descendants' | OMIT_DESCENDANTS | 'openroadm1' || 15 | 1 + 'direct descendants' | DIRECT_CHILDREN_ONLY | 'openroadm2' || 60 | 1 + 50 + 'all descendants' | INCLUDE_ALL_DESCENDANTS | 'openroadm3' || 150 | 1 + 50 * 86 } } |