Node API - GET Method performance issue

- Modified toDataNode call based on fetch descendants option.
- Used fragment extract to build fragment entity.
- Modified data set to have correct parent id for descendants.

Reviewers : Toine, Priyank and Joe

Issue-ID: CPS-1171
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>
Change-Id: I27a537fe72dd396722e6cfde7d8c454ed2579ec0
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>

3 files changed, 58 insertions, 26 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 5fe646ff8..dc848e657 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
@@ -232,22 +232,13 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         if (isRootXpath(xpath)) {
             return fragmentRepository.findFirstRootByDataspaceAndAnchor(dataspaceEntity, anchorEntity);
         } else {
-            final String normalizedXpath;
-            try {
-                normalizedXpath = CpsPathUtil.getNormalizedXpath(xpath);
-            } catch (final PathParsingException e) {
-                throw new CpsPathException(e.getMessage());
-            }
+            final String normalizedXpath = getNormalizedXpath(xpath);
             final FragmentEntity fragmentEntity;
             if (FetchDescendantsOption.OMIT_DESCENDANTS.equals(fetchDescendantsOption)) {
                 fragmentEntity =
                     fragmentRepository.getByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, normalizedXpath);
             } else {
-                final List<FragmentExtract> fragmentExtracts =
-                    fragmentRepository.findByAnchorIdAndParentXpath(anchorEntity.getId(), normalizedXpath);
-                log.debug("Fetched {} fragment entities by anchor {} and cps path {}.",
-                    fragmentExtracts.size(), anchorName, xpath);
-                fragmentEntity = FragmentEntityArranger.toFragmentEntityTree(anchorEntity, fragmentExtracts);
+                fragmentEntity = buildFragmentEntityFromFragmentExtracts(anchorEntity, normalizedXpath);
             }
             if (fragmentEntity == null) {
                 throw new DataNodeNotFoundException(dataspaceEntity.getName(), anchorEntity.getName(), xpath);
@@ -256,6 +247,17 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         }
     }
 
+    private FragmentEntity buildFragmentEntityFromFragmentExtracts(final AnchorEntity anchorEntity,
+                                                                   final String normalizedXpath) {
+        final FragmentEntity fragmentEntity;
+        final List<FragmentExtract> fragmentExtracts =
+                fragmentRepository.findByAnchorIdAndParentXpath(anchorEntity.getId(), normalizedXpath);
+        log.debug("Fetched {} fragment entities by anchor {} and cps path {}.",
+                fragmentExtracts.size(), anchorEntity.getName(), normalizedXpath);
+        fragmentEntity = FragmentEntityArranger.toFragmentEntityTree(anchorEntity, fragmentExtracts);
+        return fragmentEntity;
+    }
+
     @Override
     public List<DataNode> queryDataNodes(final String dataspaceName, final String anchorName, final String cpsPath,
                                          final FetchDescendantsOption fetchDescendantsOption) {
@@ -274,8 +276,37 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
             fragmentEntities = ancestorXpaths.isEmpty() ? Collections.emptyList()
                     : fragmentRepository.findAllByAnchorAndXpathIn(anchorEntity, ancestorXpaths);
         }
-        return fragmentEntities.stream().map(fragmentEntity -> toDataNode(fragmentEntity, fetchDescendantsOption))
-                .collect(Collectors.toUnmodifiableList());
+        return createDataNodesFromFragmentEntities(fetchDescendantsOption, anchorEntity,
+                fragmentEntities);
+    }
+
+    private List<DataNode> createDataNodesFromFragmentEntities(final FetchDescendantsOption fetchDescendantsOption,
+                                                               final AnchorEntity anchorEntity,
+                                                               final List<FragmentEntity> fragmentEntities) {
+        final List<DataNode> dataNodes = new ArrayList<>(fragmentEntities.size());
+        for (final FragmentEntity proxiedFragmentEntity : fragmentEntities) {
+            final DataNode dataNode;
+            if (FetchDescendantsOption.OMIT_DESCENDANTS.equals(fetchDescendantsOption)) {
+                dataNode = toDataNode(proxiedFragmentEntity, fetchDescendantsOption);
+            } else {
+                final String normalizedXpath = getNormalizedXpath(proxiedFragmentEntity.getXpath());
+                final FragmentEntity unproxiedFragmentEntity = buildFragmentEntityFromFragmentExtracts(anchorEntity,
+                        normalizedXpath);
+                dataNode = toDataNode(unproxiedFragmentEntity, fetchDescendantsOption);
+            }
+            dataNodes.add(dataNode);
+        }
+        return Collections.unmodifiableList(dataNodes);
+    }
+
+    private static String getNormalizedXpath(final String xpathSource) {
+        final String normalizedXpath;
+        try {
+            normalizedXpath = CpsPathUtil.getNormalizedXpath(xpathSource);
+        } catch (final PathParsingException e) {
+            throw new CpsPathException(e.getMessage());
+        }
+        return normalizedXpath;
     }
 
     @Override
diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceQueryDataNodeSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceQueryDataNodeSpec.groovy
index be2f8febf..56e388335 100644
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceQueryDataNodeSpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceQueryDataNodeSpec.groovy
@@ -149,18 +149,19 @@ class CpsDataPersistenceQueryDataNodeSpec extends CpsPersistenceSpecBase {
             result.size() == expectedXPaths.size()
             for (int i = 0; i < result.size(); i++) {
                 assert result[i].getXpath() == expectedXPaths[i]
+                assert result[i].childDataNodes.size() == expectedNumberOfChildren[i]
             }
         where: 'the following data is used'
-            scenario                                    | cpsPath                                              || expectedXPaths
-            'multiple list-ancestors'                   | '//book/ancestor::categories'                        || ["/shops/shop[@id='1']/categories[@code='1']", "/shops/shop[@id='1']/categories[@code='2']"]
-            'one ancestor with list value'              | '//book/ancestor::categories[@code=1]'               || ["/shops/shop[@id='1']/categories[@code='1']"]
-            'top ancestor'                              | '//shop[@id=1]/ancestor::shops'                      || ['/shops']
-            'list with index value in the xpath prefix' | '//categories[@code=1]/book/ancestor::shop[@id=1]'   || ["/shops/shop[@id='1']"]
-            'ancestor with parent list'                 | '//book/ancestor::shop[@id=1]/categories[@code=2]'   || ["/shops/shop[@id='1']/categories[@code='2']"]
-            'ancestor with parent'                      | '//phonenumbers[@type="mob"]/ancestor::info/contact' || ["/shops/shop[@id='3']/info/contact"]
-            'ancestor combined with text condition'     | '//book/title[text()="Dune"]/ancestor::shop'         || ["/shops/shop[@id='1']"]
-            'ancestor with parent that does not exist'  | '//book/ancestor::parentDoesNoExist/categories'      || []
-            'ancestor does not exist'                   | '//book/ancestor::ancestorDoesNotExist'              || []
+            scenario                                    | cpsPath                                              || expectedXPaths                                                                               || expectedNumberOfChildren
+            'multiple list-ancestors'                   | '//book/ancestor::categories'                        || ["/shops/shop[@id='1']/categories[@code='1']", "/shops/shop[@id='1']/categories[@code='2']"] || [1, 1]
+            'one ancestor with list value'              | '//book/ancestor::categories[@code=1]'               || ["/shops/shop[@id='1']/categories[@code='1']"]                                               || [1]
+            'top ancestor'                              | '//shop[@id=1]/ancestor::shops'                      || ['/shops']                                                                                   || [5]
+            'list with index value in the xpath prefix' | '//categories[@code=1]/book/ancestor::shop[@id=1]'   || ["/shops/shop[@id='1']"]                                                                     || [3]
+            'ancestor with parent list'                 | '//book/ancestor::shop[@id=1]/categories[@code=2]'   || ["/shops/shop[@id='1']/categories[@code='2']"]                                               || [1]
+            'ancestor with parent'                      | '//phonenumbers[@type="mob"]/ancestor::info/contact' || ["/shops/shop[@id='3']/info/contact"]                                                        || [3]
+            'ancestor combined with text condition'     | '//book/title[text()="Dune"]/ancestor::shop'         || ["/shops/shop[@id='1']"]                                                                     || [3]
+            'ancestor with parent that does not exist'  | '//book/ancestor::parentDoesNoExist/categories'      || []                                                                                           || []
+            'ancestor does not exist'                   | '//book/ancestor::ancestorDoesNotExist'              || []                                                                                           || []
     }
 
     def 'Cps Path query with syntax error throws a CPS Path Exception.'() {
diff --git a/cps-ri/src/test/resources/data/cps-path-query.sql b/cps-ri/src/test/resources/data/cps-path-query.sql
index fa711cbbf..18fd74a07 100644
--- a/cps-ri/src/test/resources/data/cps-path-query.sql
+++ b/cps-ri/src/test/resources/data/cps-path-query.sql
@@ -73,6 +73,6 @@ INSERT INTO FRAGMENT (ID, DATASPACE_ID, ANCHOR_ID, PARENT_ID, XPATH, ATTRIBUTES)
     (14, 1001, 1003, 12, '/shops/shop[@id=''3'']/categories[@code=''2'']', '{"id" : 2, "type" : "garden centre", "name": "outdoor plants"}'),
     (16, 1001, 1003, 1, '/shops/shop[@id=''3'']/info', null),
     (17, 1001, 1003, 1, '/shops/shop[@id=''3'']/info/contact', null),
-    (18, 1001, 1003, 1, '/shops/shop[@id=''3'']/info/contact/website', '{"address" : "myshop.ie"}'),
-    (19, 1001, 1003, 12, '/shops/shop[@id=''3'']/info/contact/phonenumbers[@type=''mob'']', '{"type" : "mob", "number" : "123123456"}'),
-    (20, 1001, 1003, 12, '/shops/shop[@id=''3'']/info/contact/phonenumbers[@type=''landline'']', '{"type" : "landline", "number" : "012123456"}');
+    (18, 1001, 1003, 17, '/shops/shop[@id=''3'']/info/contact/website', '{"address" : "myshop.ie"}'),
+    (19, 1001, 1003, 17, '/shops/shop[@id=''3'']/info/contact/phonenumbers[@type=''mob'']', '{"type" : "mob", "number" : "123123456"}'),
+    (20, 1001, 1003, 17, '/shops/shop[@id=''3'']/info/contact/phonenumbers[@type=''landline'']', '{"type" : "landline", "number" : "012123456"}');