Reduce anchor lookups related to PrefixResolver (CPS-2417 #1)

Instead of looking up same Anchor many times inside a for-loop,
do it once outside the loop.

This greatly improves performance in some cases, such as v2 GET API:
- /cps/api/v2/dataspaces/{dataspace}/anchors/{anchor}/node
Testing shows 3x faster response time.

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

diff --git a/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java b/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java
index 6015e0e..f86073f 100755 (executable)
--- a/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java
+++ b/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java
@@ -2,7 +2,7 @@
  *  ============LICENSE_START=======================================================
  *  Copyright (C) 2020-2022 Bell Canada.
  *  Modifications Copyright (C) 2021 Pantheon.tech
- *  Modifications Copyright (C) 2021-2023 Nordix Foundation
+ *  Modifications Copyright (C) 2021-2024 Nordix Foundation
  *  Modifications Copyright (C) 2022-2024 TechMahindra Ltd.
  *  Modifications Copyright (C) 2022 Deutsche Telekom AG
  *  ================================================================================
@@ -37,9 +37,11 @@ import java.util.List;
 import java.util.Map;
 import lombok.RequiredArgsConstructor;
 import org.apache.commons.lang3.StringUtils;
+import org.onap.cps.api.CpsAnchorService;
 import org.onap.cps.api.CpsDataService;
 import org.onap.cps.rest.api.CpsDataApi;
 import org.onap.cps.spi.FetchDescendantsOption;
+import org.onap.cps.spi.model.Anchor;
 import org.onap.cps.spi.model.DataNode;
 import org.onap.cps.spi.model.DeltaReport;
 import org.onap.cps.utils.ContentType;
@@ -63,6 +65,7 @@ public class DataRestController implements CpsDataApi {
     private static final DateTimeFormatter ISO_TIMESTAMP_FORMATTER = DateTimeFormatter.ofPattern(ISO_TIMESTAMP_FORMAT);
 
     private final CpsDataService cpsDataService;
+    private final CpsAnchorService cpsAnchorService;
     private final JsonObjectMapper jsonObjectMapper;
     private final PrefixResolver prefixResolver;
 
@@ -112,7 +115,8 @@ public class DataRestController implements CpsDataApi {
             ? FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS : FetchDescendantsOption.OMIT_DESCENDANTS;
         final DataNode dataNode = cpsDataService.getDataNodes(dataspaceName, anchorName, xpath,
             fetchDescendantsOption).iterator().next();
-        final String prefix = prefixResolver.getPrefix(dataspaceName, anchorName, dataNode.getXpath());
+        final Anchor anchor = cpsAnchorService.getAnchor(dataspaceName, anchorName);
+        final String prefix = prefixResolver.getPrefix(anchor, dataNode.getXpath());
         return new ResponseEntity<>(DataMapUtils.toDataMapWithIdentifier(dataNode, prefix), HttpStatus.OK);
     }
 
@@ -127,8 +131,9 @@ public class DataRestController implements CpsDataApi {
         final Collection<DataNode> dataNodes = cpsDataService.getDataNodes(dataspaceName, anchorName, xpath,
                 fetchDescendantsOption);
         final List<Map<String, Object>> dataMaps = new ArrayList<>(dataNodes.size());
+        final Anchor anchor = cpsAnchorService.getAnchor(dataspaceName, anchorName);
         for (final DataNode dataNode: dataNodes) {
-            final String prefix = prefixResolver.getPrefix(dataspaceName, anchorName, dataNode.getXpath());
+            final String prefix = prefixResolver.getPrefix(anchor, dataNode.getXpath());
             final Map<String, Object> dataMap = DataMapUtils.toDataMapWithIdentifier(dataNode, prefix);
             dataMaps.add(dataMap);
         }

diff --git a/cps-rest/src/main/java/org/onap/cps/rest/controller/QueryRestController.java b/cps-rest/src/main/java/org/onap/cps/rest/controller/QueryRestController.java
index 5334b48..547be66 100644 (file)
--- a/cps-rest/src/main/java/org/onap/cps/rest/controller/QueryRestController.java
+++ b/cps-rest/src/main/java/org/onap/cps/rest/controller/QueryRestController.java
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021-2022 Nordix Foundation
+ *  Copyright (C) 2021-2024 Nordix Foundation
  *  Modifications Copyright (C) 2022 Bell Canada.
  *  Modifications Copyright (C) 2022-2023 TechMahindra Ltd.
  *  ================================================================================
@@ -29,10 +29,12 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import lombok.RequiredArgsConstructor;
+import org.onap.cps.api.CpsAnchorService;
 import org.onap.cps.api.CpsQueryService;
 import org.onap.cps.rest.api.CpsQueryApi;
 import org.onap.cps.spi.FetchDescendantsOption;
 import org.onap.cps.spi.PaginationOption;
+import org.onap.cps.spi.model.Anchor;
 import org.onap.cps.spi.model.DataNode;
 import org.onap.cps.utils.DataMapUtils;
 import org.onap.cps.utils.JsonObjectMapper;
@@ -48,6 +50,7 @@ import org.springframework.web.bind.annotation.RestController;
 public class QueryRestController implements CpsQueryApi {
 
     private final CpsQueryService cpsQueryService;
+    private final CpsAnchorService cpsAnchorService;
     private final JsonObjectMapper jsonObjectMapper;
     private final PrefixResolver prefixResolver;
 
@@ -87,14 +90,15 @@ public class QueryRestController implements CpsQueryApi {
                 cpsPath, fetchDescendantsOption, paginationOption);
         final List<Map<String, Object>> dataNodesAsListOfMaps = new ArrayList<>(dataNodes.size());
         String prefix = null;
-        final Map<String, List<DataNode>> anchorDataNodeListMap = prepareDataNodesForAnchor(dataNodes);
-        for (final Map.Entry<String, List<DataNode>> anchorDataNodesMapEntry : anchorDataNodeListMap.entrySet()) {
+        final Map<String, List<DataNode>> dataNodesPerAnchor = groupDataNodesPerAnchor(dataNodes);
+        for (final Map.Entry<String, List<DataNode>> dataNodesPerAnchorEntry : dataNodesPerAnchor.entrySet()) {
+            final String anchorName = dataNodesPerAnchorEntry.getKey();
             if (prefix == null) {
-                prefix = prefixResolver.getPrefix(dataspaceName, anchorDataNodesMapEntry.getKey(),
-                        anchorDataNodesMapEntry.getValue().get(0).getXpath());
+                final Anchor anchor = cpsAnchorService.getAnchor(dataspaceName, anchorName);
+                prefix = prefixResolver.getPrefix(anchor, dataNodesPerAnchorEntry.getValue().get(0).getXpath());
             }
             final Map<String, Object> dataMap = DataMapUtils.toDataMapWithIdentifierAndAnchor(
-                    anchorDataNodesMapEntry.getValue(), anchorDataNodesMapEntry.getKey(), prefix);
+                    dataNodesPerAnchorEntry.getValue(), anchorName, prefix);
             dataNodesAsListOfMaps.add(dataMap);
         }
         final Integer totalPages = getTotalPages(dataspaceName, cpsPath, paginationOption);
@@ -112,7 +116,7 @@ public class QueryRestController implements CpsQueryApi {
                 : (int) Math.ceil((double) totalAnchors / paginationOption.getPageSize());
     }
 
-    private Map<String, List<DataNode>> prepareDataNodesForAnchor(final Collection<DataNode> dataNodes) {
+    private Map<String, List<DataNode>> groupDataNodesPerAnchor(final Collection<DataNode> dataNodes) {
         final Map<String, List<DataNode>> dataNodesMapForAnchor = new HashMap<>();
         for (final DataNode dataNode : dataNodes) {
             List<DataNode> dataNodesInAnchor = dataNodesMapForAnchor.get(dataNode.getAnchorName());
@@ -130,10 +134,11 @@ public class QueryRestController implements CpsQueryApi {
         final Collection<DataNode> dataNodes =
             cpsQueryService.queryDataNodes(dataspaceName, anchorName, cpsPath, fetchDescendantsOption);
         final List<Map<String, Object>> dataNodesAsListOfMaps = new ArrayList<>(dataNodes.size());
+        final Anchor anchor = cpsAnchorService.getAnchor(dataspaceName, anchorName);
         String prefix = null;
         for (final DataNode dataNode : dataNodes) {
             if (prefix == null) {
-                prefix = prefixResolver.getPrefix(dataspaceName, anchorName, dataNode.getXpath());
+                prefix = prefixResolver.getPrefix(anchor, dataNode.getXpath());
             }
             final Map<String, Object> dataMap = DataMapUtils.toDataMapWithIdentifier(dataNode, prefix);
             dataNodesAsListOfMaps.add(dataMap);

diff --git a/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy b/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy
index 5241f61..e101ea6 100755 (executable)
--- a/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy
+++ b/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021-2022 Nordix Foundation
+ *  Copyright (C) 2021-2024 Nordix Foundation
  *  Modifications Copyright (C) 2021 Pantheon.tech
  *  Modifications Copyright (C) 2021-2022 Bell Canada.
  *  Modifications Copyright (C) 2022 Deutsche Telekom AG
@@ -26,6 +26,7 @@ package org.onap.cps.rest.controller
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import groovy.json.JsonSlurper
+import org.onap.cps.api.CpsAnchorService
 import org.onap.cps.api.CpsDataService
 import org.onap.cps.spi.FetchDescendantsOption
 import org.onap.cps.spi.model.DataNode
@@ -62,6 +63,9 @@ class DataRestControllerSpec extends Specification {
     @SpringBean
     CpsDataService mockCpsDataService = Mock()
 
+    @SpringBean
+    CpsAnchorService mockCpsAnchorService = Mock()
+
     @SpringBean
     JsonObjectMapper jsonObjectMapper = new JsonObjectMapper(new ObjectMapper())
 

diff --git a/cps-rest/src/test/groovy/org/onap/cps/rest/controller/QueryRestControllerSpec.groovy b/cps-rest/src/test/groovy/org/onap/cps/rest/controller/QueryRestControllerSpec.groovy
index c30a63f..80b287c 100644 (file)
--- a/cps-rest/src/test/groovy/org/onap/cps/rest/controller/QueryRestControllerSpec.groovy
+++ b/cps-rest/src/test/groovy/org/onap/cps/rest/controller/QueryRestControllerSpec.groovy
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021-2022 Nordix Foundation
+ *  Copyright (C) 2021-2024 Nordix Foundation
  *  Modifications Copyright (C) 2021-2022 Bell Canada.
  *  Modifications Copyright (C) 2021 Pantheon.tech
  *  Modifications Copyright (C) 2022-2023 TechMahindra Ltd.
@@ -23,18 +23,13 @@
 
 package org.onap.cps.rest.controller
 
-import org.onap.cps.spi.PaginationOption
-import org.onap.cps.utils.PrefixResolver
-
-import static org.onap.cps.spi.FetchDescendantsOption.DIRECT_CHILDREN_ONLY
-import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS
-import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
-
 import com.fasterxml.jackson.databind.ObjectMapper
-import org.onap.cps.utils.JsonObjectMapper
+import org.onap.cps.api.CpsAnchorService
 import org.onap.cps.api.CpsQueryService
+import org.onap.cps.spi.PaginationOption
 import org.onap.cps.spi.model.DataNodeBuilder
+import org.onap.cps.utils.JsonObjectMapper
+import org.onap.cps.utils.PrefixResolver
 import org.spockframework.spring.SpringBean
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.beans.factory.annotation.Value
@@ -43,12 +38,20 @@ import org.springframework.http.HttpStatus
 import org.springframework.test.web.servlet.MockMvc
 import spock.lang.Specification
 
+import static org.onap.cps.spi.FetchDescendantsOption.DIRECT_CHILDREN_ONLY
+import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS
+import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
+
 @WebMvcTest(QueryRestController)
 class QueryRestControllerSpec extends Specification {
 
     @SpringBean
     CpsQueryService mockCpsQueryService = Mock()
 
+    @SpringBean
+    CpsAnchorService mockCpsAnchorService = Mock()
+
     @SpringBean
     JsonObjectMapper jsonObjectMapper = new JsonObjectMapper(new ObjectMapper())
 

diff --git a/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java b/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java
index 951770b..eed4f09 100644 (file)
--- a/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java
+++ b/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java
@@ -400,8 +400,7 @@ public class CpsDataServiceImpl implements CpsDataService {
     private List<Map<String, Object>> prefixResolver(final Anchor anchor, final Collection<DataNode> dataNodes) {
         final List<Map<String, Object>> prefixToDataNodes = new ArrayList<>(dataNodes.size());
         for (final DataNode dataNode: dataNodes) {
-            final String prefix = prefixResolver
-                    .getPrefix(anchor.getDataspaceName(), anchor.getName(), dataNode.getXpath());
+            final String prefix = prefixResolver.getPrefix(anchor, dataNode.getXpath());
             final Map<String, Object> prefixToDataNode = DataMapUtils.toDataMapWithIdentifier(dataNode, prefix);
             prefixToDataNodes.add(prefixToDataNode);
         }

diff --git a/cps-service/src/main/java/org/onap/cps/utils/PrefixResolver.java b/cps-service/src/main/java/org/onap/cps/utils/PrefixResolver.java
index 35dc734..93fb728 100644 (file)
--- a/cps-service/src/main/java/org/onap/cps/utils/PrefixResolver.java
+++ b/cps-service/src/main/java/org/onap/cps/utils/PrefixResolver.java
@@ -54,19 +54,6 @@ public class PrefixResolver {
 
     private final IMap<String, AnchorDataCacheEntry> anchorDataCache;
 
-    /**
-     * Get the module prefix for the given xpath for a dataspace and anchor name.
-     *
-     * @param dataspaceName the name of the dataspace
-     * @param anchorName the name of the anchor the xpath belongs to
-     * @param xpath the xpath to prefix a prefix for
-     * @return the prefix of the module the top level element of given xpath
-     */
-    public String getPrefix(final String dataspaceName, final String anchorName, final String xpath) {
-        final Anchor anchor = cpsAnchorService.getAnchor(dataspaceName, anchorName);
-        return getPrefix(anchor, xpath);
-    }
-
     /**
      * Get the module prefix for the given xpath under the given anchor.
      *

diff --git a/cps-service/src/test/groovy/org/onap/cps/utils/PrefixResolverSpec.groovy b/cps-service/src/test/groovy/org/onap/cps/utils/PrefixResolverSpec.groovy
index b975de6..5ef584a 100644 (file)
--- a/cps-service/src/test/groovy/org/onap/cps/utils/PrefixResolverSpec.groovy
+++ b/cps-service/src/test/groovy/org/onap/cps/utils/PrefixResolverSpec.groovy
@@ -48,10 +48,10 @@ class PrefixResolverSpec extends Specification {
 
     def schemaContext = YangTextSchemaSourceSetBuilder.of(yangResourceNameToContent).getSchemaContext()
 
+    def anchor = new Anchor(dataspaceName: 'testDataspace', name: 'testAnchor')
+
     def setup() {
-        given: 'an anchor for the test-tree model'
-            def anchor = new Anchor(dataspaceName: 'testDataspace', name: 'testAnchor')
-        and: 'the system can get this anchor'
+        given: 'the system can get the anchor'
             mockCpsAnchorService.getAnchor('testDataspace', 'testAnchor') >> anchor
         and: 'the schema source cache contains the schema context for the test-tree module'
             mockYangTextSchemaSourceSet.getSchemaContext() >> schemaContext
@@ -59,7 +59,7 @@ class PrefixResolverSpec extends Specification {
 
     def 'get xpath prefix using node schema context'() {
         when: 'the prefix of the yang module is retrieved'
-            def result = objectUnderTest.getPrefix('testDataspace', 'testAnchor', xpath)
+            def result = objectUnderTest.getPrefix(anchor, xpath)
         then: 'the expected prefix is returned'
             result == expectedPrefix
         and: 'the cache is updated for the given anchor with a map of prefixes per top level container (just one one this case)'
@@ -90,7 +90,7 @@ class PrefixResolverSpec extends Specification {
             mockAnchorDataCache.containsKey('testAnchor') >> true
             mockAnchorDataCache.get('testAnchor') >> anchorDataCacheEntry
         when: 'the prefix of the yang module is retrieved'
-            def result = objectUnderTest.getPrefix('testDataspace', 'testAnchor', '/test-tree')
+            def result = objectUnderTest.getPrefix(anchor, '/test-tree')
         then: 'the expected prefix is returned'
             result == expectedPrefix
         and: 'schema source cache is not used (i.e. no need to build schema context)'
@@ -108,7 +108,7 @@ class PrefixResolverSpec extends Specification {
             mockAnchorDataCache.containsKey('testAnchor') >> true
             mockAnchorDataCache.get('testAnchor') >> anchorDataCacheEntry
         when: 'the prefix of the yang module is retrieved'
-            def result = objectUnderTest.getPrefix('testDataspace', 'testAnchor', '/test-tree')
+            def result = objectUnderTest.getPrefix(anchor, '/test-tree')
         then: 'the expected prefix is returned'
             result == 'tree'
         and: 'schema source cache is used (i.e. need to build schema context)'