From f92d32193545e0b2cd8f9124b01433678b813435 Mon Sep 17 00:00:00 2001 From: ToineSiebelink Date: Thu, 15 Apr 2021 12:15:01 +0100 Subject: Fix "ends-with" query syntax to conform with xPath definition "ends-with" is HOW we resolve it in sql query. 'descendant anywhere' is the correct Path name for the '//' operator - Updated method names, variable names, test description to reflect the correct terminolgy - Udpated query to always perfix the target (descendant name) with an '\' so it alwasy only matches whole node names - Updated regex for cpsPath to NOT accept triple /// (as per xPath this is invalid since a ndoeName cannot start with or contain a node separator Issue-ID: CPS-336 Signed-off-by: ToineSiebelink Change-Id: I9f181d6986d038311b839e3f9c5afc4237c7d347 --- .../spi/impl/CpsDataPersistenceServiceImpl.java | 2 +- .../java/org/onap/cps/spi/query/CpsPathQuery.java | 10 +++---- .../org/onap/cps/spi/query/CpsPathQueryType.java | 4 +-- .../cps/spi/repository/FragmentRepository.java | 12 ++++---- .../spi/impl/CpsDataPersistenceServiceSpec.groovy | 32 +++++++++++----------- .../org/onap/cps/spi/query/CpsPathQuerySpec.groovy | 20 ++++++++------ 6 files changed, 42 insertions(+), 38 deletions(-) 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 26aa4ac0b..ca279f30c 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 @@ -145,7 +145,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService .getLeafName(), cpsPathQuery.getLeafValue()); } else { fragmentEntities = fragmentRepository - .getByAnchorAndEndsWithXpath(anchorEntity.getId(), cpsPathQuery.getEndsWith()); + .getByAnchorAndXpathEndsInDescendantName(anchorEntity.getId(), cpsPathQuery.getDescendantName()); } return fragmentEntities.stream() .map(fragmentEntity -> toDataNode(fragmentEntity, fetchDescendantsOption)) diff --git a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java index 97a304d76..ce78c06d4 100644 --- a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQuery.java @@ -35,7 +35,7 @@ public class CpsPathQuery { private String xpathPrefix; private String leafName; private Object leafValue; - private String endsWith; + private String descendantName; private static final String NON_CAPTURING_GROUP_1_TO_99_YANG_CONTAINERS = "((?:\\/[^\\/]+){1,99})"; @@ -45,7 +45,7 @@ public class CpsPathQuery { private static final Pattern QUERY_CPS_PATH_WITH_SINGLE_LEAF_PATTERN = Pattern.compile(NON_CAPTURING_GROUP_1_TO_99_YANG_CONTAINERS + YANG_LEAF_VALUE_EQUALS_CONDITION); - private static final Pattern QUERY_CPS_PATH_ENDS_WITH_PATTERN = Pattern.compile("\\/\\/(.+)"); + private static final Pattern DESCENDANT_ANYWHERE_PATTERN = Pattern.compile("\\/\\/([^\\/].+)"); private static final Pattern LEAF_INTEGER_VALUE_PATTERN = Pattern.compile("[-+]?\\d+"); @@ -67,10 +67,10 @@ public class CpsPathQuery { cpsPathQuery.setLeafValue(convertLeafValueToCorrectType(matcher.group(3), cpsPath)); return cpsPathQuery; } - matcher = QUERY_CPS_PATH_ENDS_WITH_PATTERN.matcher(cpsPath); + matcher = DESCENDANT_ANYWHERE_PATTERN.matcher(cpsPath); if (matcher.matches()) { - cpsPathQuery.setCpsPathQueryType(CpsPathQueryType.XPATH_ENDS_WITH); - cpsPathQuery.setEndsWith(matcher.group(1)); + cpsPathQuery.setCpsPathQueryType(CpsPathQueryType.XPATH_HAS_DESCENDANT_ANYWHERE); + cpsPathQuery.setDescendantName(matcher.group(1)); return cpsPathQuery; } throw new CpsPathException("Invalid cps path.", diff --git a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQueryType.java b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQueryType.java index 1a0f8cabc..abdbfb9fd 100644 --- a/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQueryType.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/query/CpsPathQueryType.java @@ -24,9 +24,9 @@ package org.onap.cps.spi.query; */ public enum CpsPathQueryType { /** - * Xpath ends with cps path query type e.g. //cps-path . + * Xpath descendant anywhere type e.g. //nodeName . */ - XPATH_ENDS_WITH, + XPATH_HAS_DESCENDANT_ANYWHERE, /** * Xpath leaf value cps path query type e.g. /cps-path[@leafName=leafValue] . */ diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java index 5ff7cfc97..eb2e82b6c 100755 --- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java @@ -1,6 +1,6 @@ /*- * ============LICENSE_START======================================================= - * Copyright (C) 2020 Nordix Foundation. All rights reserved. + * Copyright (C) 2020-201 Nordix Foundation. All rights reserved. * Modifications Copyright (C) 2020 Bell Canada. All rights reserved. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); @@ -61,7 +61,9 @@ public interface FragmentRepository extends JpaRepository List getByAnchorAndXpathAndLeafAttributes(@Param("anchor") int anchorId, @Param("xpath") String xpathPrefix, @Param("leafName") String leafName, @Param("leafValue") Object leafValue); - @Query(value = "SELECT * FROM FRAGMENT WHERE anchor_id = :anchor AND xpath LIKE %:xpath", nativeQuery = true) - // Above query will match the end of an xpath and anchor id - List getByAnchorAndEndsWithXpath(@Param("anchor") int anchorId, @Param("xpath") String xpath); -} \ No newline at end of file + @Query(value = "SELECT * FROM FRAGMENT WHERE anchor_id = :anchor AND xpath LIKE CONCAT('%/',:descendantName)", + nativeQuery = true) + // Above query will match the anchor id and last descendant name + List getByAnchorAndXpathEndsInDescendantName(@Param("anchor") int anchorId, + @Param("descendantName") String descendantName); +} diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy index 231a57283..a2c7a0947 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy @@ -338,9 +338,9 @@ class CpsDataPersistenceServiceSpec extends CpsPersistenceSpecBase { dataNode.getLeaves().toString() == leaves dataNode.getChildDataNodes().size() == expectedNumberOfChidlNodes where: 'the following data is used' - type | cpsPath | includeDescendantsOption | expectedNumberOfChidlNodes - 'String and no descendants' | '/parent-200/child-202[@common-leaf-name=\'common-leaf-value\']' | OMIT_DESCENDANTS | 0 - 'Integer and descendants' | '/parent-200/child-202[@common-leaf-name-int=5]' | INCLUDE_ALL_DESCENDANTS | 1 + type | cpsPath | includeDescendantsOption || expectedNumberOfChidlNodes + 'String and no descendants' | '/parent-200/child-202[@common-leaf-name=\'common-leaf-value\']' | OMIT_DESCENDANTS || 0 + 'Integer and descendants' | '/parent-200/child-202[@common-leaf-name-int=5]' | INCLUDE_ALL_DESCENDANTS || 1 } @Unroll @@ -355,35 +355,35 @@ class CpsDataPersistenceServiceSpec extends CpsPersistenceSpecBase { 'cps path is incomplete' | '/parent-200[@common-leaf-name-int=5]' 'leaf value does not exist' | '/parent-200/child-202[@common-leaf-name=\'does not exist\']' 'incomplete end of xpath prefix' | '/parent-200/child-20[@common-leaf-name-int=5]' - 'empty cps path of type ends with' | '///' } @Unroll @Sql([CLEAR_DATA, SET_DATA]) - def 'Cps Path query with and without descendants using #type.'() { + def 'Cps Path query using descendant anywhere and #type (further) descendants.'() { when: 'a query is executed to get a data node by the given cps path' - def cpsPath = '///child-202' + def cpsPath = '//child-202' def result = objectUnderTest.queryDataNodes(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, cpsPath, includeDescendantsOption) then: 'the data node has the correct number of children' DataNode dataNode = result.stream().findFirst().get() dataNode.getChildDataNodes().size() == expectedNumberOfChildNodes where: 'the following data is used' - type | includeDescendantsOption | expectedNumberOfChildNodes - 'ends with and omit descendants' | OMIT_DESCENDANTS | 0 - 'ends with and include all descendants' | INCLUDE_ALL_DESCENDANTS | 1 + type | includeDescendantsOption || expectedNumberOfChildNodes + 'omit' | OMIT_DESCENDANTS || 0 + 'include' | INCLUDE_ALL_DESCENDANTS || 1 } @Unroll @Sql([CLEAR_DATA, SET_DATA]) - def 'Cps Path query using ends with variations of #type.'() { + def 'Cps Path query using descendant anywhere with %scenario '() { when: 'a query is executed to get a data node by the given cps path' def result = objectUnderTest.queryDataNodes(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, cpsPath, OMIT_DESCENDANTS) - then: 'the correct number of data nodes is returned' - result.size() == expectedNumberOfDataNodes + then: 'Only one data node is returned' + result.size() == 1 + and: + result.stream().findFirst().get().xpath == expectedXPath where: 'the following data is used' - type | cpsPath | expectedNumberOfDataNodes - 'single match with / prefix' | '///child-202' | 1 - 'single match without / prefix' | '//grand-child-202' | 1 - 'multiple matches' | '//202' | 2 + scenario | cpsPath || expectedXPath + 'fully unique descendant name' | '//grand-child-202' || '/parent-200/child-202/grand-child-202' + 'descendant name match end of other node' | '//child-202' || '/parent-200/child-202' } } diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy index 0bc99c644..7f558dd10 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/query/CpsPathQuerySpec.groovy @@ -50,13 +50,14 @@ class CpsPathQuerySpec extends Specification { when: 'the given cps path is parsed' def result = objectUnderTest.createFrom(cpsPath) then: 'the query has the right type' - result.cpsPathQueryType == CpsPathQueryType.XPATH_ENDS_WITH + result.cpsPathQueryType == CpsPathQueryType.XPATH_HAS_DESCENDANT_ANYWHERE and: 'the right ends with parameters are set' - result.endsWith == expectedEndsWithValue + result.descendantName == expectedEndsWithValue where: 'the following data is used' - scenario | cpsPath || expectedEndsWithValue - 'yang container' | '///cps-path' || '/cps-path' - 'yang list' | '///cps-path[@key=value]' || '/cps-path[@key=value]' + scenario | cpsPath || expectedEndsWithValue + 'yang container' | '//cps-path' || 'cps-path' + 'yang list' | '//cps-path[@key=value]' || 'cps-path[@key=value]' + 'parent & child' | '//parent/child' || 'parent/child' } @Unroll @@ -66,9 +67,10 @@ class CpsPathQuerySpec extends Specification { then: 'a CpsPathException is thrown' thrown(CpsPathException) where: 'the following data is used' - scenario | cpsPath - 'no / at the start' | 'invalid-cps-path/child' - 'float value' | '/parent-200/child-202[@common-leaf-name-float=5.0]' - 'too many containers' | '/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17/18/19/20/21/22/23/24/25/26/27/28/29/30/31/32/33/34/35/36/37/38/39/40/41/42/43/44/45/46/47/48/49/50/51/52/53/54/55/56/57/58/59/60/61/62/63/64/65/66/67/68/69/70/71/72/73/74/75/76/77/78/79/80/81/82/83/84/85/86/87/88/89/90/91/92/93/94/95/96/97/98/99/100[@a=1]' + scenario | cpsPath + 'no / at the start' | 'invalid-cps-path/child' + 'additional / after descendant option' | '///cps-path' + 'float value' | '/parent-200/child-202[@common-leaf-name-float=5.0]' + 'too many containers' | '/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17/18/19/20/21/22/23/24/25/26/27/28/29/30/31/32/33/34/35/36/37/38/39/40/41/42/43/44/45/46/47/48/49/50/51/52/53/54/55/56/57/58/59/60/61/62/63/64/65/66/67/68/69/70/71/72/73/74/75/76/77/78/79/80/81/82/83/84/85/86/87/88/89/90/91/92/93/94/95/96/97/98/99/100[@a=1]' } } -- cgit 1.2.3-korg