Allow getDataNodes to read whole lists(ep 1)

 - getDataNodes can now retrieve list data nodes.

Issue-ID: CPS-1696
Signed-off-by: arpitsingh <as00745003@techmahindra.com>
Change-Id: I320a368d6cb73599d3f7c13fe9b8ab7d0cc69470

6 files changed, 102 insertions, 53 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 f4afe3d73b..f904e8bddd 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
@@ -120,7 +120,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         newChildAsFragmentEntity.setParentId(parentFragmentEntity.getId());
         try {
             fragmentRepository.save(newChildAsFragmentEntity);
-        } catch (final DataIntegrityViolationException e) {
+        } catch (final DataIntegrityViolationException dataIntegrityViolationException) {
             throw AlreadyDefinedException.forDataNodes(Collections.singletonList(newChild.getXpath()),
                     anchorEntity.getName());
         }
@@ -138,9 +138,9 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
                 fragmentEntities.add(newChildAsFragmentEntity);
             }
             fragmentRepository.saveAll(fragmentEntities);
-        } catch (final DataIntegrityViolationException e) {
+        } catch (final DataIntegrityViolationException dataIntegrityViolationException) {
             log.warn("Exception occurred : {} , While saving : {} children, retrying using individual save operations",
-                    e, fragmentEntities.size());
+                    dataIntegrityViolationException, fragmentEntities.size());
             retrySavingEachChildIndividually(anchorEntity, parentNodeXpath, newChildren);
         }
     }
@@ -151,7 +151,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         for (final DataNode newChild : newChildren) {
             try {
                 addNewChildDataNode(anchorEntity, parentNodeXpath, newChild);
-            } catch (final AlreadyDefinedException e) {
+            } catch (final AlreadyDefinedException alreadyDefinedException) {
                 failedXpaths.add(newChild.getXpath());
             }
         }
@@ -184,7 +184,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
             try {
                 final FragmentEntity fragmentEntity = convertToFragmentWithAllDescendants(anchorEntity, dataNode);
                 fragmentRepository.save(fragmentEntity);
-            } catch (final DataIntegrityViolationException e) {
+            } catch (final DataIntegrityViolationException dataIntegrityViolationException) {
                 failedXpaths.add(dataNode.getXpath());
             }
         }
@@ -251,22 +251,28 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
 
     private Collection<FragmentEntity> getFragmentEntities(final AnchorEntity anchorEntity,
                                                            final Collection<String> xpaths) {
-        final Collection<String> nonRootXpaths = new HashSet<>(xpaths);
-        final boolean haveRootXpath = nonRootXpaths.removeIf(CpsDataPersistenceServiceImpl::isRootXpath);
+        final Collection<String> normalizedXpaths = getNormalizedXpaths(xpaths);
 
-        final Collection<String> normalizedXpaths = new HashSet<>(nonRootXpaths.size());
-        for (final String xpath : nonRootXpaths) {
-            try {
-                normalizedXpaths.add(CpsPathUtil.getNormalizedXpath(xpath));
-            } catch (final PathParsingException e) {
-                log.warn("Error parsing xpath \"{}\": {}", xpath, e.getMessage());
+        final boolean haveRootXpath = normalizedXpaths.removeIf(CpsDataPersistenceServiceImpl::isRootXpath);
+
+        final List<FragmentEntity> fragmentEntities = fragmentRepository.findByAnchorAndXpathIn(anchorEntity,
+                normalizedXpaths);
+
+        for (final FragmentEntity fragmentEntity : fragmentEntities) {
+            normalizedXpaths.remove(fragmentEntity.getXpath());
+        }
+
+        for (final String xpath : normalizedXpaths) {
+            if (!CpsPathUtil.isPathToListElement(xpath)) {
+                fragmentEntities.addAll(fragmentRepository.findListByAnchorAndXpath(anchorEntity, xpath));
             }
         }
+
         if (haveRootXpath) {
-            normalizedXpaths.addAll(fragmentRepository.findAllXpathByAnchorAndParentIdIsNull(anchorEntity));
+            fragmentEntities.addAll(fragmentRepository.findRootsByAnchorId(anchorEntity.getId()));
         }
 
-        return fragmentRepository.findByAnchorAndXpathIn(anchorEntity, normalizedXpaths);
+        return fragmentEntities;
     }
 
     private FragmentEntity getFragmentEntity(final AnchorEntity anchorEntity, final String xpath) {
@@ -293,8 +299,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         final CpsPathQuery cpsPathQuery;
         try {
             cpsPathQuery = CpsPathUtil.getCpsPathQuery(cpsPath);
-        } catch (final PathParsingException e) {
-            throw new CpsPathException(e.getMessage());
+        } catch (final PathParsingException pathParsingException) {
+            throw new CpsPathException(pathParsingException.getMessage());
         }
 
         Collection<FragmentEntity> fragmentEntities;
@@ -337,11 +343,23 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         }
         try {
             return CpsPathUtil.getNormalizedXpath(xpathSource);
-        } catch (final PathParsingException e) {
-            throw new CpsPathException(e.getMessage());
+        } catch (final PathParsingException pathParsingException) {
+            throw new CpsPathException(pathParsingException.getMessage());
         }
     }
 
+    private static Collection<String> getNormalizedXpaths(final Collection<String> xpaths) {
+        final Collection<String> normalizedXpaths = new HashSet<>(xpaths.size());
+        for (final String xpath : xpaths) {
+            try {
+                normalizedXpaths.add(getNormalizedXpath(xpath));
+            } catch (final CpsPathException cpsPathException) {
+                log.warn("Error parsing xpath \"{}\": {}", xpath, cpsPathException.getMessage());
+            }
+        }
+        return normalizedXpaths;
+    }
+
     @Override
     public String startSession() {
         return sessionManager.startSession();
@@ -450,7 +468,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         for (final FragmentEntity dataNodeFragment : fragmentEntities) {
             try {
                 fragmentRepository.save(dataNodeFragment);
-            } catch (final StaleStateException e) {
+            } catch (final StaleStateException staleStateException) {
                 failedXpaths.add(dataNodeFragment.getXpath());
             }
         }
@@ -542,15 +560,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
 
         final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName);
 
-        final Collection<String> deleteChecklist = new HashSet<>(xpathsToDelete.size());
-        for (final String xpath : xpathsToDelete) {
-            try {
-                deleteChecklist.add(CpsPathUtil.getNormalizedXpath(xpath));
-            } catch (final PathParsingException e) {
-                log.warn("Error parsing xpath \"{}\": {}", xpath, e.getMessage());
-            }
-        }
-
+        final Collection<String> deleteChecklist = getNormalizedXpaths(xpathsToDelete);
         final Collection<String> xpathsToExistingContainers =
             fragmentRepository.findAllXpathByAnchorAndXpathIn(anchorEntity, deleteChecklist);
         if (onlySupportListDeletion) {
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 303af5bc47..7d5be13a5d 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
@@ -58,6 +58,17 @@ public interface FragmentRepository extends JpaRepository<FragmentEntity, Long>,
         return findByAnchorIdAndXpathIn(anchorEntity.getId(), xpaths.toArray(new String[0]));

     }

 

+    @Query(value = "SELECT * FROM fragment WHERE anchor_id = :anchorId \n"

+            + "AND xpath LIKE :escapedXpath||'[@%]' AND xpath NOT LIKE :escapedXpath||'[@%]/%[@%]'",

+            nativeQuery = true)

+    List<FragmentEntity> findListByAnchorIdAndEscapedXpath(@Param("anchorId") long anchorId,

+                                                           @Param("escapedXpath") String escapedXpath);

+

+    default List<FragmentEntity> findListByAnchorAndXpath(final AnchorEntity anchorEntity, final String xpath) {

+        final String escapedXpath = EscapeUtils.escapeForSqlLike(xpath);

+        return findListByAnchorIdAndEscapedXpath(anchorEntity.getId(), escapedXpath);

+    }

+

     @Query(value = "SELECT fragment.* FROM fragment JOIN anchor ON anchor.id = fragment.anchor_id "

         + "WHERE dataspace_id = :dataspaceId AND xpath = ANY (:xpaths)", nativeQuery = true)

     List<FragmentEntity> findByDataspaceIdAndXpathIn(@Param("dataspaceId") int dataspaceId,

@@ -110,7 +121,7 @@ public interface FragmentRepository extends JpaRepository<FragmentEntity, Long>,
 

     boolean existsByAnchorAndXpathStartsWith(AnchorEntity anchorEntity, String xpath);

 

-    @Query("SELECT xpath FROM FragmentEntity WHERE anchor = :anchor AND parentId IS NULL")

-    List<String> findAllXpathByAnchorAndParentIdIsNull(@Param("anchor") AnchorEntity anchorEntity);

+    @Query(value = "SELECT * FROM fragment WHERE anchor_id = :anchorId AND parent_id IS NULL", nativeQuery = true)

+    List<FragmentEntity> findRootsByAnchorId(@Param("anchorId") long anchorId);

 

 }

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 cb554faee8..c72c3046e8 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
@@ -56,6 +56,7 @@ class CpsDataPersistenceServiceSpec extends Specification {
     def setup() {
         mockAnchorRepository.getByDataspaceAndName(_, _) >> anchorEntity
         mockFragmentRepository.prefetchDescendantsOfFragmentEntities(_, _) >> { fetchDescendantsOption, fragmentEntities -> fragmentEntities }
+        mockFragmentRepository.findListByAnchorAndXpath(_, [] as Set) >> []
     }
 
     def 'Storing data nodes individually when batch operation fails'(){
diff --git a/docs/release-notes.rst b/docs/release-notes.rst
index 6b35461422..66dde1cfb7 100755
--- a/docs/release-notes.rst
+++ b/docs/release-notes.rst
@@ -42,7 +42,7 @@ Bug Fixes
 
 Features
 --------
-3.3.6
+    - `CPS-1696 <https://jira.onap.org/browse/CPS-1696>`_ Get Data Node to return entire List data node.
 
 
 Version: 3.3.5
diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsDataServiceIntegrationSpec.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsDataServiceIntegrationSpec.groovy
index a3f14397c2..6b556d3bc6 100644
--- a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsDataServiceIntegrationSpec.groovy
+++ b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsDataServiceIntegrationSpec.groovy
@@ -113,23 +113,49 @@ class CpsDataServiceIntegrationSpec extends FunctionalSpecBase {
             restoreBookstoreDataAnchor(1)
     }
 
+    def 'Get whole list data' () {
+            def xpathForWholeList = "/bookstore/categories"
+        when: 'get data nodes for bookstore container'
+            def dataNodes = objectUnderTest.getDataNodes(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, xpathForWholeList, FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS)
+        then: 'the tree consist ouf of #expectNumberOfDataNodes data nodes'
+            assert dataNodes.size() == 5
+        and: 'each datanode contains the list node xpath partially in its xpath'
+            dataNodes.each {dataNode ->
+                assert dataNode.xpath.contains(xpathForWholeList)
+            }
+    }
+
+    def 'Read (multiple) data nodes with #scenario' () {
+        when: 'attempt to get data nodes using multiple valid xpaths'
+            def dataNodes = objectUnderTest.getDataNodesForMultipleXpaths(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, xpath, OMIT_DESCENDANTS)
+        then: 'expected numer of data nodes are returned'
+            dataNodes.size() == expectedNumberOfDataNodes
+        where: 'the following data was used'
+                    scenario                    |                       xpath                                       |   expectedNumberOfDataNodes
+            'container-node xpath'              | ['/bookstore']                                                    |               1
+            'list-item'                         | ['/bookstore/categories[@code=1]']                                |               1
+            'parent-list xpath'                 | ['/bookstore/categories']                                         |               5
+            'child-list xpath'                  | ['/bookstore/categories[@code=1]/books']                          |               2
+            'both parent and child list xpath'  | ['/bookstore/categories', '/bookstore/categories[@code=1]/books'] |               7
+    }
+
     def 'Add and Delete a (container) data node using #scenario.'() {
-        when: 'the new datanode is saved'
-            objectUnderTest.saveData(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1 , parentXpath, json, now)
-        then: 'it can be retrieved by its normalized xpath'
-            def result = objectUnderTest.getDataNodes(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, normalizedXpathToNode, DIRECT_CHILDREN_ONLY)
-            assert result.size() == 1
-            assert result[0].xpath == normalizedXpathToNode
-        and: 'there is now one extra datanode'
-            assert originalCountBookstoreChildNodes + 1 == countDataNodesInBookstore()
-        when: 'the new datanode is deleted'
-            objectUnderTest.deleteDataNode(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, normalizedXpathToNode, now)
-        then: 'the original number of data nodes is restored'
-            assert originalCountBookstoreChildNodes == countDataNodesInBookstore()
-        where:
-            scenario                      | parentXpath                         | json                                                                                        || normalizedXpathToNode
-            'normalized parent xpath'     | '/bookstore'                        | '{"webinfo": {"domain-name":"ourbookstore.com", "contact-email":"info@ourbookstore.com" }}' || "/bookstore/webinfo"
-            'non-normalized parent xpath' | '/bookstore/categories[ @code="1"]' | '{"books": {"title":"new" }}'                                                               || "/bookstore/categories[@code='1']/books[@title='new']"
+            when: 'the new datanode is saved'
+                objectUnderTest.saveData(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1 , parentXpath, json, now)
+            then: 'it can be retrieved by its normalized xpath'
+                def result = objectUnderTest.getDataNodes(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, normalizedXpathToNode, DIRECT_CHILDREN_ONLY)
+                assert result.size() == 1
+                assert result[0].xpath == normalizedXpathToNode
+            and: 'there is now one extra datanode'
+                assert originalCountBookstoreChildNodes + 1 == countDataNodesInBookstore()
+            when: 'the new datanode is deleted'
+                objectUnderTest.deleteDataNode(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, normalizedXpathToNode, now)
+            then: 'the original number of data nodes is restored'
+                assert originalCountBookstoreChildNodes == countDataNodesInBookstore()
+            where:
+                scenario                      | parentXpath                         | json                                                                                        || normalizedXpathToNode
+                'normalized parent xpath'     | '/bookstore'                        | '{"webinfo": {"domain-name":"ourbookstore.com", "contact-email":"info@ourbookstore.com" }}' || "/bookstore/webinfo"
+                'non-normalized parent xpath' | '/bookstore/categories[ @code="1"]' | '{"books": {"title":"new" }}'                                                               || "/bookstore/categories[@code='1']/books[@title='new']"
     }
 
     def 'Attempt to create a top level data node using root.'() {
diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/GetPerfTest.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/GetPerfTest.groovy
index eee87dd7c0..8323ef9f84 100644
--- a/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/GetPerfTest.groovy
+++ b/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/GetPerfTest.groovy
@@ -74,11 +74,12 @@ class GetPerfTest extends CpsPerfTestBase {
         then: 'all data is read within #durationLimit ms'
             recordAndAssertPerformance("Read datatrees using ${scenario}", durationLimit, durationInMillis)
         where: 'the following xpaths are used'
-            scenario                | anchorPrefix | xpath                || durationLimit | expectedNumberOfDataNodes
-            'bookstore root'        | 'bookstore'  | '/'                  || 200           | 78
-            'bookstore top element' | 'bookstore'  | '/bookstore'         || 200           | 78
-            'openroadm root'        | 'openroadm'  | '/'                  || 600           | 1 + 50 * 86
-            'openroadm top element' | 'openroadm'  | '/openroadm-devices' || 600           | 1 + 50 * 86
+            scenario                | anchorPrefix | xpath                                  || durationLimit | expectedNumberOfDataNodes
+            'bookstore root'        | 'bookstore'  | '/'                                    || 200           | 78
+            'bookstore top element' | 'bookstore'  | '/bookstore'                           || 200           | 78
+            'openroadm root'        | 'openroadm'  | '/'                                    || 600           | 1 + 50 * 86
+            'openroadm top element' | 'openroadm'  | '/openroadm-devices'                   || 600           | 1 + 50 * 86
+            'openroadm whole list'  | 'openroadm'  | '/openroadm-devices/openroadm-device'  || 600           | 50 * 86
     }
 
 }