Use getDataNodes (plural version) into NCMP to get CM handles Performance improvement

- Code clean-up on existing classes
- Removed duplicate code

Issue-ID: CPS-1426
Signed-off-by: seanbeirne <sean.beirne@est.tech>
Change-Id: I3e2ce5b804d7a6e87838490d970cde3cd563f06b

5 files changed, 42 insertions, 114 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java
index f8e06593c5..7466308981 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java
@@ -91,40 +91,6 @@ public class YangDataConverter {
     }
 
     /**
-     * This method convert cm handle data node to yang model cm handle without using NcmpServiceCmHandle.
-     *
-     * @param cmHandleDataNode the datanode of the cm handle
-     * @param cmHandleId       the id of the cm handle
-     * @return yang model cm handle
-     */
-    public static YangModelCmHandle convertCmHandleToYangModelWithoutNcmpServiceCmHandle(
-            final DataNode cmHandleDataNode,
-            final String cmHandleId) {
-        final Map<String, String> dmiProperties = new LinkedHashMap<>();
-        final Map<String, String> publicProperties = new LinkedHashMap<>();
-        final CompositeStateBuilder compositeStateBuilder = new CompositeStateBuilder();
-        CompositeState compositeState = compositeStateBuilder.build();
-        for (final DataNode childDataNode : cmHandleDataNode.getChildDataNodes()) {
-            if (childDataNode.getXpath().contains("/additional-properties[@name=")) {
-                addProperty(childDataNode, dmiProperties);
-            } else if (childDataNode.getXpath().contains("/public-properties[@name=")) {
-                addProperty(childDataNode, publicProperties);
-            } else if (childDataNode.getXpath().endsWith("/state")) {
-                compositeState = compositeStateBuilder.fromDataNode(childDataNode).build();
-            }
-        }
-        return YangModelCmHandle.toYangModelCmHandleWithoutNcmpServiceHandle(
-                (String) cmHandleDataNode.getLeaves().get("dmi-service-name"),
-                (String) cmHandleDataNode.getLeaves().get("dmi-data-service-name"),
-                (String) cmHandleDataNode.getLeaves().get("dmi-model-service-name"),
-                cmHandleId,
-                dmiProperties,
-                publicProperties,
-                compositeState
-        );
-    }
-
-    /**
      * This method convert cm handle data nodes to yang model cm handles.
      * @param cmHandleDataNodes the datanode of the cm handle
      * @return yang model cm handles
@@ -134,21 +100,15 @@ public class YangDataConverter {
         final Collection<YangModelCmHandle> yangModelCmHandles = new ArrayList<>(cmHandleDataNodes.size());
         cmHandleDataNodes.forEach(dataNode -> {
             final String cmHandleId = extractCmHandleIdFromXpath(dataNode.getXpath());
-            if (cmHandleId != null) {
-                yangModelCmHandles.add(convertCmHandleToYangModelWithoutNcmpServiceCmHandle(dataNode, cmHandleId));
-            }
+            yangModelCmHandles.add(convertCmHandleToYangModel(dataNode, cmHandleId));
         });
         return yangModelCmHandles;
     }
 
     private static String extractCmHandleIdFromXpath(final String xpath) {
         final Matcher matcher = cmHandleIdInXpathPattern.matcher(xpath);
-        if (matcher.find()) {
-            return matcher.group(1);
-        } else {
-            log.error("Unexpected xpath {}", xpath);
-        }
-        return null;
+        matcher.find();
+        return matcher.group(1);
     }
 
 
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/yangmodels/YangModelCmHandle.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/yangmodels/YangModelCmHandle.java
index 5f98f14995..a6f953aa86 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/yangmodels/YangModelCmHandle.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/yangmodels/YangModelCmHandle.java
@@ -115,38 +115,6 @@ public class YangModelCmHandle {
         return yangModelCmHandle;
     }
 
-
-    /**
-     * Create a yangModelCmHandle without the use of NcmpServiceHandle.
-     *
-     * @param dmiServiceName      dmi service name
-     * @param dmiDataServiceName  dmi data service name
-     * @param dmiModelServiceName dmi model service name
-     * @param cmHandleId          the cm handle id
-     * @param dmiProperties       dmiProperties
-     * @param publicProperties    publicProperties
-     * @param compositeState      compositeState
-     * @return instance of yangModelCmHandle
-     */
-    public static YangModelCmHandle toYangModelCmHandleWithoutNcmpServiceHandle(final String dmiServiceName,
-                                                        final String dmiDataServiceName,
-                                                        final String dmiModelServiceName,
-                                                        final String cmHandleId,
-                                                        final Map<String, String> dmiProperties,
-                                                        final Map<String, String> publicProperties,
-                                                        final CompositeState compositeState) {
-        final YangModelCmHandle yangModelCmHandle = new YangModelCmHandle();
-        yangModelCmHandle.setId(cmHandleId);
-        yangModelCmHandle.setDmiServiceName(dmiServiceName);
-        yangModelCmHandle.setDmiDataServiceName(dmiDataServiceName);
-        yangModelCmHandle.setDmiModelServiceName(dmiModelServiceName);
-        yangModelCmHandle.setDmiProperties(asYangModelCmHandleProperties(dmiProperties));
-        yangModelCmHandle.setPublicProperties(asYangModelCmHandleProperties(publicProperties));
-        yangModelCmHandle.setCompositeState(compositeState);
-        return yangModelCmHandle;
-    }
-
-
     /**
      * Resolve a dmi service name.
      *
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImpl.java
index c2578ccbf7..4d1202b065 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImpl.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImpl.java
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2022 Nordix Foundation
+ *  Copyright (C) 2022-2023 Nordix Foundation
  *  Modifications Copyright (C) 2022 Bell Canada
  *  ================================================================================
  *  Licensed under the Apache License, Version 2.0 (the "License");
@@ -33,7 +33,6 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.onap.cps.api.CpsAdminService;
@@ -62,8 +61,6 @@ public class InventoryPersistenceImpl implements InventoryPersistence {
 
     private static final String NCMP_DMI_REGISTRY_PARENT = "/dmi-registry";
 
-    private static final String CM_HANDLE_XPATH_TEMPLATE = "/dmi-registry/cm-handles[@id='" + "%s" + "']";
-
     private final JsonObjectMapper jsonObjectMapper;
 
     private final CpsDataService cpsDataService;
@@ -77,7 +74,7 @@ public class InventoryPersistenceImpl implements InventoryPersistence {
     @Override
     public CompositeState getCmHandleState(final String cmHandleId) {
         final DataNode stateAsDataNode = cpsDataService.getDataNode(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR,
-                String.format(CM_HANDLE_XPATH_TEMPLATE, cmHandleId) + "/state",
+                createCmHandleXPath(cmHandleId) + "/state",
                 FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS);
         cpsValidator.validateNameCharacters(cmHandleId);
         return new CompositeStateBuilder().fromDataNode(stateAsDataNode).build();
@@ -85,10 +82,9 @@ public class InventoryPersistenceImpl implements InventoryPersistence {
 
     @Override
     public void saveCmHandleState(final String cmHandleId, final CompositeState compositeState) {
-        final String cmHandleJsonData = String.format("{\"state\":%s}",
-                jsonObjectMapper.asJsonString(compositeState));
+        final String cmHandleJsonData = createStateJsonData(jsonObjectMapper.asJsonString(compositeState));
         cpsDataService.updateDataNodeAndDescendants(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR,
-                String.format(CM_HANDLE_XPATH_TEMPLATE, cmHandleId),
+                createCmHandleXPath(cmHandleId),
                 cmHandleJsonData, OffsetDateTime.now());
     }
 
@@ -96,8 +92,8 @@ public class InventoryPersistenceImpl implements InventoryPersistence {
     public void saveCmHandleStateBatch(final Map<String, CompositeState> cmHandleStatePerCmHandleId) {
         final Map<String, String> cmHandlesJsonDataMap = new HashMap<>();
         cmHandleStatePerCmHandleId.forEach((cmHandleId, compositeState) -> cmHandlesJsonDataMap.put(
-                String.format(CM_HANDLE_XPATH_TEMPLATE, cmHandleId),
-                String.format("{\"state\":%s}", jsonObjectMapper.asJsonString(compositeState))));
+                createCmHandleXPath(cmHandleId),
+                createStateJsonData(jsonObjectMapper.asJsonString(compositeState))));
         cpsDataService.updateDataNodesAndDescendants(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR,
                 cmHandlesJsonDataMap, OffsetDateTime.now());
     }
@@ -110,7 +106,7 @@ public class InventoryPersistenceImpl implements InventoryPersistence {
 
     @Override
     public Collection<YangModelCmHandle> getYangModelCmHandles(final Collection<String> cmHandleIds) {
-        final Collection<String> validCmHandleIds = new ArrayList<>();
+        final Collection<String> validCmHandleIds = new ArrayList<>(cmHandleIds.size());
         cmHandleIds.forEach(cmHandleId -> {
             try {
                 cpsValidator.validateNameCharacters(cmHandleId);
@@ -137,7 +133,7 @@ public class InventoryPersistenceImpl implements InventoryPersistence {
     @Override
     public void saveCmHandle(final YangModelCmHandle yangModelCmHandle) {
         final String cmHandleJsonData =
-                String.format("{\"cm-handles\":[%s]}", jsonObjectMapper.asJsonString(yangModelCmHandle));
+                createCmHandleJsonData(jsonObjectMapper.asJsonString(yangModelCmHandle));
         cpsDataService.saveListElements(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, NCMP_DMI_REGISTRY_PARENT,
                 cmHandleJsonData, NO_TIMESTAMP);
     }
@@ -146,7 +142,7 @@ public class InventoryPersistenceImpl implements InventoryPersistence {
     public void saveCmHandleBatch(final Collection<YangModelCmHandle> yangModelCmHandles) {
         final List<String> cmHandlesJsonData = new ArrayList<>();
         yangModelCmHandles.forEach(yangModelCmHandle -> cmHandlesJsonData.add(
-                String.format("{\"cm-handles\":[%s]}", jsonObjectMapper.asJsonString(yangModelCmHandle))));
+                createCmHandleJsonData(jsonObjectMapper.asJsonString(yangModelCmHandle))));
         cpsDataService.saveListElementsBatch(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR,
                 NCMP_DMI_REGISTRY_PARENT, cmHandlesJsonData, NO_TIMESTAMP);
     }
@@ -199,14 +195,13 @@ public class InventoryPersistenceImpl implements InventoryPersistence {
 
     @Override
     public DataNode getCmHandleDataNode(final String cmHandleId) {
-        return this.getDataNode(String.format(CM_HANDLE_XPATH_TEMPLATE, cmHandleId));
+        return this.getDataNode(createCmHandleXPath(cmHandleId));
     }
 
     @Override
     public Collection<DataNode> getCmHandleDataNodes(final Collection<String> cmHandleIds) {
-        final Collection<String> xpaths = cmHandleIds.stream().map(cmHandleId ->
-                        String.format(CM_HANDLE_XPATH_TEMPLATE, cmHandleId))
-                .collect(Collectors.toList());
+        final Collection<String> xpaths = new ArrayList<>(cmHandleIds.size());
+        cmHandleIds.forEach(cmHandleId -> xpaths.add(createCmHandleXPath(cmHandleId)));
         return this.getDataNodes(xpaths);
     }
 
@@ -230,4 +225,16 @@ public class InventoryPersistenceImpl implements InventoryPersistence {
     public void deleteDataNodes(final Collection<String> dataNodeXpaths) {
         cpsDataService.deleteDataNodes(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, dataNodeXpaths, NO_TIMESTAMP);
     }
+
+    private static String createCmHandleXPath(final String cmHandleId) {
+        return "/dmi-registry/cm-handles[@id='" + cmHandleId + "']";
+    }
+
+    private static String createStateJsonData(final String state) {
+        return "{\"state\":" + state + "}";
+    }
+
+    private static String createCmHandleJsonData(final String cmHandleId) {
+        return "{\"cm-handles\":[" + cmHandleId + "]}";
+    }
 }
\ No newline at end of file
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/YangDataConverterSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/YangDataConverterSpec.groovy
index dd673eb8f8..6d733dc475 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/YangDataConverterSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/YangDataConverterSpec.groovy
@@ -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.
@@ -25,31 +25,33 @@ import spock.lang.Specification
 
 class YangDataConverterSpec extends Specification{
 
-    def 'Convert a cm handle data node with private properties.'() {
-        given: 'a datanode with some additional (dmi, private) properties'
+    def 'Convert a cm handle data node with private and public properties.'() {
+        given: 'a datanode with some additional (dmi, private) and public properties'
             def dataNodeAdditionalProperties = new DataNode(xpath:'/additional-properties[@name="dmiProp1"]',
-                leaves: ['name': 'dmiProp1', 'value': 'dmiValue1'])
-            def dataNode = new DataNode(childDataNodes:[dataNodeAdditionalProperties])
+                    leaves: ['name': 'dmiProp1', 'value': 'dmiValue1'])
+            def dataNodePublicProperties = new DataNode(xpath:'/public-properties[@name="pubProp1"]',
+                    leaves: ['name': 'pubProp1', 'value': 'pubValue1'])
+            def dataNodeCmHandle = new DataNode(childDataNodes:[dataNodeAdditionalProperties, dataNodePublicProperties])
         when: 'the dataNode is converted'
-            def yangModelCmHandle = YangDataConverter.convertCmHandleToYangModel(dataNode,'sample-id')
+            def yangModelCmHandle = YangDataConverter.convertCmHandleToYangModel(dataNodeCmHandle,'sample-id')
         then: 'the converted object has the correct id'
             assert yangModelCmHandle.id == 'sample-id'
         and: 'the additional (dmi, private) properties are included'
             assert yangModelCmHandle.dmiProperties[0].name == 'dmiProp1'
             assert yangModelCmHandle.dmiProperties[0].value == 'dmiValue1'
+        and: 'the public properties are included'
+            assert yangModelCmHandle.publicProperties[0].name == 'pubProp1'
+            assert yangModelCmHandle.publicProperties[0].value == 'pubValue1'
     }
 
     def 'Convert multiple cm handle data nodes'(){
-        given: 'two data nodes in a collection one with private properties'
-            def dataNodeAdditionalProperties = new DataNode(xpath:'/additional-properties[@name="dmiProp1"]',
-                    leaves: ['name': 'dmiProp1', 'value': 'dmiValue1'])
+        given: 'two data nodes in a collection'
             def dataNodes = [new DataNode(xpath:'/dmi-registry/cm-handles[@id=\'some-cm-handle\']'),
-                             new DataNode(xpath:'/dmi-registry/cm-handles[@id=\'another-cm-handle\']', childDataNodes:[dataNodeAdditionalProperties])]
+                             new DataNode(xpath:'/dmi-registry/cm-handles[@id=\'another-cm-handle\']')]
         when: 'the data nodes are converted'
             def yangModelCmHandles = YangDataConverter.convertDataNodesToYangModelCmHandles(dataNodes)
-        then: 'verify both have returned and cmhandleIds are correct'
+        then: 'verify both have returned and CmHandleIds are correct'
             assert yangModelCmHandles.size() == 2
             assert yangModelCmHandles.id.containsAll(['some-cm-handle', 'another-cm-handle'])
     }
-
 }
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImplSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImplSpec.groovy
index 93e79f6824..929ea84e36 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImplSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceImplSpec.groovy
@@ -134,15 +134,6 @@ class InventoryPersistenceImplSpec extends Specification {
             assert results.id.containsAll([cmHandleId, cmHandleId2])
     }
 
-    def "Handling name validation errors in getYangModelCmHandles."() {
-        given: 'the cps data service returns one of two data nodes from the DMI registry with empty leaf attributes'
-            mockCpsValidator.validateNameCharacters(cmHandleId) >> {throw new DataValidationException('some message', 'some detail')}
-        when:
-            objectUnderTest.getYangModelCmHandle(cmHandleId)
-        then: 'exception is thrown'
-            thrown(DataValidationException)
-    }
-
     def 'Get a Cm Handle Composite State'() {
         given: 'a valid cm handle id'
             def cmHandleId = 'Some-Cm-Handle'
@@ -300,7 +291,7 @@ class InventoryPersistenceImplSpec extends Specification {
             objectUnderTest.deleteDataNodes(['xpath1', 'xpath2'])
         then: 'the cps data service method to delete data nodes is invoked once with the same xPaths'
             1 * mockCpsDataService.deleteDataNodes('NCMP-Admin', 'ncmp-dmi-registry',
-                ['xpath1', 'xpath2'], NO_TIMESTAMP);
+                    ['xpath1', 'xpath2'], NO_TIMESTAMP);
     }
 
 }