summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordanielhanrahan <daniel.hanrahan@est.tech>2023-06-23 14:37:12 +0100
committerdanielhanrahan <daniel.hanrahan@est.tech>2023-07-10 12:20:30 +0100
commite05a59a2361394d6fc4a93193b0ed35ba305fcf8 (patch)
treecc5a48a39bdee3a66bb82161a9d4a6e8b5868a99
parent5769fae8b2cf4246bb2fd079dfd7b8db8130fcc0 (diff)
Handle special characters in CpsPath queries (CPS-1760 #2)
This fixes issues with special characters for CPS-500, CPS-1756, CPS-1758, and CPS-1760. It also improves query performance. - use SQL LIKE instead of regex in Cps Path queries Issue-ID: CPS-1763 Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech> Change-Id: I5c179882bfba71d3b009c60059e9073f46227e7d
-rw-r--r--cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java32
-rw-r--r--cps-ri/src/test/groovy/org/onap/cps/spi/utils/EscapeUtilsSpec.groovy4
-rw-r--r--docs/cps-path.rst3
-rw-r--r--integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy30
-rw-r--r--integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/QueryPerfTest.groovy28
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
}
}