Normalize xpaths for getDataNodes

Issue-ID: CPS-1457
Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech>
Change-Id: I93d19666c168aa69da73eadbfef0fc54181aec52

5 files changed, 74 insertions, 24 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 5b0683e97..06ee8ecad 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
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021-2022 Nordix Foundation
+ *  Copyright (C) 2021-2023 Nordix Foundation
  *  Modifications Copyright (C) 2021 Pantheon.tech
  *  Modifications Copyright (C) 2020-2022 Bell Canada.
  *  Modifications Copyright (C) 2022 TechMahindra Ltd.
@@ -262,13 +262,19 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
                                              final FetchDescendantsOption fetchDescendantsOption) {
         final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName);
         final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName);
-        final List<FragmentEntity> fragmentEntities =
-                fragmentRepository.findByAnchorAndMultipleCpsPaths(anchorEntity.getId(), xpaths);
-        final Collection<DataNode> dataNodesCollection = new ArrayList<>(fragmentEntities.size());
-        for (final FragmentEntity fragmentEntity : fragmentEntities) {
-            dataNodesCollection.add(toDataNode(fragmentEntity, fetchDescendantsOption));
+
+        final Set<String> normalizedXpaths = new HashSet<>(xpaths.size());
+        for (final String xpath : xpaths) {
+            try {
+                normalizedXpaths.add(CpsPathUtil.getNormalizedXpath(xpath));
+            } catch (final PathParsingException e) {
+                log.warn("Error parsing xpath \"{}\" in getDataNodes: {}", xpath, e.getMessage());
+            }
         }
-        return dataNodesCollection;
+
+        final List<FragmentEntity> fragmentEntities =
+                fragmentRepository.findByAnchorAndMultipleCpsPaths(anchorEntity.getId(), normalizedXpaths);
+        return toDataNodes(fragmentEntities, fetchDescendantsOption);
     }
 
     private FragmentEntity getFragmentWithoutDescendantsByXpath(final String dataspaceName,
@@ -449,6 +455,15 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
                 .withChildDataNodes(childDataNodes).build();
     }
 
+    private Collection<DataNode> toDataNodes(final Collection<FragmentEntity> fragmentEntities,
+                                             final FetchDescendantsOption fetchDescendantsOption) {
+        final Collection<DataNode> dataNodes = new ArrayList<>(fragmentEntities.size());
+        for (final FragmentEntity fragmentEntity : fragmentEntities) {
+            dataNodes.add(toDataNode(fragmentEntity, fetchDescendantsOption));
+        }
+        return dataNodes;
+    }
+
     private List<DataNode> getChildDataNodes(final FragmentEntity fragmentEntity,
                                              final FetchDescendantsOption fetchDescendantsOption) {
         if (fetchDescendantsOption.hasNext()) {
diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepositoryMultiPathQueryImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepositoryMultiPathQueryImpl.java
index b936e5c76..8c357bbb3 100644
--- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepositoryMultiPathQueryImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepositoryMultiPathQueryImpl.java
@@ -1,6 +1,6 @@
 /*-
  * ============LICENSE_START=======================================================
- *  Copyright (C) 2022 Nordix Foundation.
+ *  Copyright (C) 2022-2023 Nordix Foundation.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@ package org.onap.cps.spi.repository;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import javax.persistence.EntityManager;
@@ -46,6 +47,9 @@ public class FragmentRepositoryMultiPathQueryImpl implements FragmentRepositoryM
     @Transactional
     public List<FragmentEntity> findByAnchorAndMultipleCpsPaths(final Integer anchorId,
                                                                 final Collection<String> cpsPathQueryList) {
+        if (cpsPathQueryList.isEmpty()) {
+            return Collections.emptyList();
+        }
         final Collection<List<String>> sqlData = new HashSet<>(cpsPathQueryList.size());
         for (final String query : cpsPathQueryList) {
             final List<String> row = new ArrayList<>(1);
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 cc2369d50..6252fff56 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
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021-2022 Nordix Foundation
+ *  Copyright (C) 2021-2023 Nordix Foundation
  *  Modifications Copyright (C) 2021 Pantheon.tech
  *  Modifications Copyright (C) 2021-2022 Bell Canada.
  *  Modifications Copyright (C) 2022 TechMahindra Ltd.
@@ -296,6 +296,39 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
+    def 'Get multiple data nodes by xpath.'() {
+        when: 'fetch #scenario.'
+            def results = objectUnderTest.getDataNodes(DATASPACE_NAME, ANCHOR_NAME3, inputXpaths, OMIT_DESCENDANTS)
+        then: 'the expected number of data nodes are returned'
+            assert results.size() == expectedResultSize
+        where: 'following parameters were used'
+            scenario                               | inputXpaths                                     || expectedResultSize
+            '1 node'                               | ["/parent-200"]                                 || 1
+            '2 unique nodes'                       | ["/parent-200", "/parent-201"]                  || 2
+            '3 unique nodes'                       | ["/parent-200", "/parent-201", "/parent-202"]   || 3
+            '1 unique node with duplicate xpath'   | ["/parent-200", "/parent-200"]                  || 1
+            '2 unique nodes with duplicate xpath'  | ["/parent-200", "/parent-202", "/parent-200"]   || 2
+            'list element with key (single quote)' | ["/parent-201/child-204[@key='A']"]             || 1
+            'list element with key (double quote)' | ['/parent-201/child-204[@key="A"]']             || 1
+            'non-existing xpath'                   | ["/NO-XPATH"]                                   || 0
+            'existing and non-existing xpaths'     | ["/parent-200", "/NO-XPATH", "/parent-201"]     || 2
+            'invalid xpath'                        | ["INVALID XPATH"]                               || 0
+            'valid and invalid xpaths'             | ["/parent-200", "INVALID XPATH", "/parent-201"] || 2
+    }
+
+    @Sql([CLEAR_DATA, SET_DATA])
+    def 'Get multiple data nodes error scenario: #scenario.'() {
+        when: 'attempt to get data nodes with #scenario'
+            objectUnderTest.getDataNodes(dataspaceName, anchorName, ['/not-relevant'], OMIT_DESCENDANTS)
+        then: 'a #expectedException is thrown'
+            thrown(expectedException)
+        where: 'the following data is used'
+            scenario                 | dataspaceName  | anchorName     || expectedException
+            'non-existing dataspace' | 'NO DATASPACE' | 'not relevant' || DataspaceNotFoundException
+            'non-existing anchor'    | DATASPACE_NAME | 'NO ANCHOR'    || AnchorNotFoundException
+    }
+
+    @Sql([CLEAR_DATA, SET_DATA])
     def 'Update data node leaves.'() {
         when: 'update is performed for leaves'
             objectUnderTest.updateDataLeaves(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES,
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 01c1dc70e..87e59c60d 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
@@ -1,8 +1,8 @@
 /*
  * ============LICENSE_START=======================================================
  * Copyright (c) 2021 Bell Canada.
- * Modifications Copyright (C) 2021-2022 Nordix Foundation
- *  Modifications Copyright (C) 2022 TechMahindra Ltd.
+ * Modifications Copyright (C) 2021-2023 Nordix Foundation
+ * Modifications Copyright (C) 2022 TechMahindra Ltd.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -146,11 +146,11 @@ class CpsDataPersistenceServiceSpec extends Specification {
            def anchorEntity = new AnchorEntity(id:123)
            mockAnchorRepository.getByDataspaceAndName(*_) >> anchorEntity
         and: 'fragment repository returns a collection of fragments'
-            def fragmentEntity1 = new FragmentEntity(xpath: 'xpath1', childFragments: [])
-            def fragmentEntity2 = new FragmentEntity(xpath: 'xpath2', childFragments: [])
-           mockFragmentRepository.findByAnchorAndMultipleCpsPaths(123, ['xpath1','xpath2']) >> [ fragmentEntity1, fragmentEntity2 ]
+            def fragmentEntity1 = new FragmentEntity(xpath: '/xpath1', childFragments: [])
+            def fragmentEntity2 = new FragmentEntity(xpath: '/xpath2', childFragments: [])
+            mockFragmentRepository.findByAnchorAndMultipleCpsPaths(123, ['/xpath1', '/xpath2'] as Set<String>) >> [fragmentEntity1, fragmentEntity2]
         when: 'getting data nodes for 2 xpaths'
-            def result = objectUnderTest.getDataNodes('some-dataspace', 'some-anchor', ['xpath1','xpath2'],FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS)
+            def result = objectUnderTest.getDataNodes('some-dataspace', 'some-anchor', ['/xpath1', '/xpath2'], FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS)
         then: '2 data nodes are returned'
             assert result.size() == 2
     }
diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy
index 28363d74b..2346239df 100644
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy
@@ -51,8 +51,6 @@ class CpsDataPersistenceServicePerfTest extends CpsPersistencePerfSpecBase {
     static def NUMBER_OF_CHILDREN = 200
     static def NUMBER_OF_GRAND_CHILDREN = 50
     static def TOTAL_NUMBER_OF_NODES = 1 + NUMBER_OF_CHILDREN + (NUMBER_OF_CHILDREN * NUMBER_OF_GRAND_CHILDREN)  //  Parent + Children +  Grand-children
-    static def ALLOWED_SETUP_TIME_MS = TimeUnit.SECONDS.toMillis(10)
-    static def ALLOWED_READ_TIME_AL_NODES_MS = 500
 
     def stopWatch = new StopWatch()
     def readStopWatch = new StopWatch()
@@ -64,8 +62,8 @@ class CpsDataPersistenceServicePerfTest extends CpsPersistencePerfSpecBase {
             createLineage(objectUnderTest, NUMBER_OF_CHILDREN, NUMBER_OF_GRAND_CHILDREN, false)
             stopWatch.stop()
             def setupDurationInMillis = stopWatch.getTotalTimeMillis()
-        and: 'setup duration is under #ALLOWED_SETUP_TIME_MS milliseconds'
-            assert setupDurationInMillis < ALLOWED_SETUP_TIME_MS
+        and: 'setup duration is under 10 seconds'
+            assert setupDurationInMillis < 10000
     }
 
     def 'Get data node with many descendants by xpath #scenario'() {
@@ -75,7 +73,7 @@ class CpsDataPersistenceServicePerfTest extends CpsPersistencePerfSpecBase {
             stopWatch.stop()
             def readDurationInMillis = stopWatch.getTotalTimeMillis()
         then: 'read duration is under 500 milliseconds'
-            assert readDurationInMillis < ALLOWED_READ_TIME_AL_NODES_MS
+            assert readDurationInMillis < 500
         and: 'data node is returned with all the descendants populated'
             assert countDataNodes(result) == TOTAL_NUMBER_OF_NODES
         where: 'the following xPaths are used'
@@ -91,7 +89,7 @@ class CpsDataPersistenceServicePerfTest extends CpsPersistencePerfSpecBase {
             stopWatch.stop()
             def readDurationInMillis = stopWatch.getTotalTimeMillis()
         then: 'read duration is under 500 milliseconds'
-            assert readDurationInMillis < ALLOWED_READ_TIME_AL_NODES_MS
+            assert readDurationInMillis < 500
         and: 'data node is returned with all the descendants populated'
             assert countDataNodes(result) == TOTAL_NUMBER_OF_NODES
     }
@@ -115,12 +113,12 @@ class CpsDataPersistenceServicePerfTest extends CpsPersistencePerfSpecBase {
             def result = objectUnderTest.queryDataNodes(PERF_DATASPACE, PERF_ANCHOR,  '//perf-test-grand-child-1', descendantsOption)
             stopWatch.stop()
             def readDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'read duration is under 500 milliseconds'
-            assert readDurationInMillis < alowedDuration
+        then: 'read duration is under #allowedDuration milliseconds'
+            assert readDurationInMillis < allowedDuration
         and: 'data node is returned with all the descendants populated'
             assert result.size() == NUMBER_OF_CHILDREN
         where: 'the following options are used'
-            scenario                                        | descendantsOption        || alowedDuration
+            scenario                                        | descendantsOption        || allowedDuration
             'omit descendants                             ' | OMIT_DESCENDANTS         || 150
             'include descendants (although there are none)' | INCLUDE_ALL_DESCENDANTS  || 150
     }