Faster alternate-id checks during registration (CPS-2366 #2)

This improves performance when using alternate ID in registration.
Instead of one CPS path query for each alternate ID, it sends one
query for the whole batch using OR operator, e.g.

  /dmi-registry/cm-handles[@alternate-id='A' or @alternate-id='B']

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

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/AlternateIdChecker.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/AlternateIdChecker.java
index f8e13b7..3600d6d 100644 (file)
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/AlternateIdChecker.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/AlternateIdChecker.java
@@ -22,8 +22,8 @@ package org.onap.cps.ncmp.impl.inventory;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.Set;
+import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
@@ -55,13 +55,13 @@ public class AlternateIdChecker {
     public Collection<String> getIdsOfCmHandlesWithRejectedAlternateId(
                                     final Collection<NcmpServiceCmHandle> newNcmpServiceCmHandles,
                                     final Operation operation) {
-        final Set<String> acceptedAlternateIds = new HashSet<>(newNcmpServiceCmHandles.size());
+        final Set<String> assignedAlternateIds = getAlternateIdsAlreadyInDb(newNcmpServiceCmHandles);
         final Collection<String> rejectedCmHandleIds = new ArrayList<>();
         for (final NcmpServiceCmHandle ncmpServiceCmHandle : newNcmpServiceCmHandles) {
             final String cmHandleId = ncmpServiceCmHandle.getCmHandleId();
             final String proposedAlternateId = ncmpServiceCmHandle.getAlternateId();
-            if (isProposedAlternateIdAcceptable(proposedAlternateId, operation, acceptedAlternateIds, cmHandleId)) {
-                acceptedAlternateIds.add(proposedAlternateId);
+            if (isProposedAlternateIdAcceptable(proposedAlternateId, operation, assignedAlternateIds, cmHandleId)) {
+                assignedAlternateIds.add(proposedAlternateId);
             } else {
                 rejectedCmHandleIds.add(cmHandleId);
             }
@@ -70,15 +70,10 @@ public class AlternateIdChecker {
     }
 
     private boolean isProposedAlternateIdAcceptable(final String proposedAlternateId, final Operation operation,
-                                                    final Set<String> acceptedAlternateIds, final String cmHandleId) {
+                                                    final Set<String> assignedAlternateIds, final String cmHandleId) {
         if (StringUtils.isBlank(proposedAlternateId)) {
             return true;
         }
-        if (acceptedAlternateIds.contains(proposedAlternateId)) {
-            log.warn("Alternate id update ignored, cannot update cm handle {}, alternate id is already "
-                + "assigned to a different cm handle (in this batch)", cmHandleId);
-            return false;
-        }
         final String currentAlternateId = getCurrentAlternateId(operation, cmHandleId);
         if (currentAlternateId.equals(proposedAlternateId)) {
             return true;
@@ -88,7 +83,7 @@ public class AlternateIdChecker {
                     cmHandleId, currentAlternateId);
             return false;
         }
-        if (alternateIdAlreadyInDb(proposedAlternateId)) {
+        if (assignedAlternateIds.contains(proposedAlternateId)) {
             log.warn("Alternate id update ignored, cannot update cm handle {}, alternate id is already "
                     + "assigned to a different cm handle", cmHandleId);
             return false;
@@ -96,13 +91,14 @@ public class AlternateIdChecker {
         return true;
     }
 
-    private boolean alternateIdAlreadyInDb(final String alternateId) {
-        try {
-            inventoryPersistence.getCmHandleDataNodeByAlternateId(alternateId);
-        } catch (final DataNodeNotFoundException dataNodeNotFoundException) {
-            return false;
-        }
-        return true;
+    private Set<String> getAlternateIdsAlreadyInDb(final Collection<NcmpServiceCmHandle> ncmpServiceCmHandles) {
+        final Set<String> alternateIdsToCheck = ncmpServiceCmHandles.stream()
+                .map(NcmpServiceCmHandle::getAlternateId)
+                .filter(StringUtils::isNotBlank)
+                .collect(Collectors.toSet());
+        return inventoryPersistence.getCmHandleDataNodesByAlternateIds(alternateIdsToCheck).stream()
+                .map(dataNode -> (String) dataNode.getLeaves().get("alternate-id"))
+                .collect(Collectors.toSet());
     }
 
     private String getCurrentAlternateId(final Operation operation, final String cmHandleId) {

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistence.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistence.java
index beef752..a0d3a3e 100644 (file)
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistence.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistence.java
@@ -129,6 +129,14 @@ public interface InventoryPersistence extends NcmpPersistence {
      */
     DataNode getCmHandleDataNodeByAlternateId(String alternateId);
 
+    /**
+     * Get data nodes for the given batch of alternate ids.
+     *
+     * @param alternateIds alternate IDs
+     * @return data nodes
+     */
+    Collection<DataNode> getCmHandleDataNodesByAlternateIds(Collection<String> alternateIds);
+
     /**
      * Get collection of data nodes of given cm handles.
      *

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImpl.java
index 083b25d..42123f1 100644 (file)
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImpl.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImpl.java
@@ -32,6 +32,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
 import org.onap.cps.api.CpsAnchorService;
 import org.onap.cps.api.CpsDataService;
@@ -183,6 +184,15 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv
         return dataNodes.iterator().next();
     }
 
+    @Override
+    public Collection<DataNode> getCmHandleDataNodesByAlternateIds(final Collection<String> alternateIds) {
+        if (alternateIds.isEmpty()) {
+            return Collections.emptyList();
+        }
+        final String cpsPathForCmHandlesByAlternateIds = getCpsPathForCmHandlesByAlternateIds(alternateIds);
+        return cmHandleQueryService.queryNcmpRegistryByCpsPath(cpsPathForCmHandlesByAlternateIds, OMIT_DESCENDANTS);
+    }
+
     @Override
     public Collection<DataNode> getCmHandleDataNodes(final Collection<String> cmHandleIds) {
         final Collection<String> xpaths = new ArrayList<>(cmHandleIds.size());
@@ -212,6 +222,11 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv
         return NCMP_DMI_REGISTRY_PARENT + "/cm-handles[@alternate-id='" + alternateId + "']";
     }
 
+    private static String getCpsPathForCmHandlesByAlternateIds(final Collection<String> alternateIds) {
+        return alternateIds.stream().collect(Collectors.joining("' or @alternate-id='",
+                NCMP_DMI_REGISTRY_PARENT + "/cm-handles[@alternate-id='", "']"));
+    }
+
     private static String createStateJsonData(final String state) {
         return "{\"state\":" + state + "}";
     }

diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/AlternateIdCheckerSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/AlternateIdCheckerSpec.groovy
index 6642532..b976ff4 100644 (file)
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/AlternateIdCheckerSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/AlternateIdCheckerSpec.groovy
@@ -37,8 +37,8 @@ class AlternateIdCheckerSpec extends Specification {
             def batch = [new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: alt1),
                          new NcmpServiceCmHandle(cmHandleId: 'ch-2', alternateId: alt2)]
         and: 'the database already contains cm handle(s) with these alternate ids: #altAlreadyInDb'
-            mockInventoryPersistenceService.getCmHandleDataNodeByAlternateId(_) >>
-                {  args -> altAlreadyInDb.contains(args[0]) ? new DataNode() : throwDataNodeNotFoundException() }
+            mockInventoryPersistenceService.getCmHandleDataNodesByAlternateIds(_ as Collection<String>) >>
+                { args -> args[0].stream().filter(altId -> altAlreadyInDb.contains(altId)).map(altId -> new DataNode(leaves: ["alternate-id": altId])).toList() }
         when: 'the batch of new cm handles is checked'
             def result = objectUnderTest.getIdsOfCmHandlesWithRejectedAlternateId(batch, AlternateIdChecker.Operation.CREATE)
         then: 'the result contains ids of the rejected cm handles'
@@ -57,8 +57,8 @@ class AlternateIdCheckerSpec extends Specification {
         given: 'a batch of 1 existing cm handle to update alternate id to #proposedAlt'
             def batch = [new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: proposedAlt)]
         and: 'the database already contains a cm handle with alternate id: #altAlreadyInDb'
-            mockInventoryPersistenceService.getCmHandleDataNodeByAlternateId(_) >>
-                    {  args -> altAlreadyInDb.equals(args[0]) ? new DataNode() : throwDataNodeNotFoundException() }
+            mockInventoryPersistenceService.getCmHandleDataNodesByAlternateIds(_ as Collection<String>) >>
+                { args -> args[0].stream().filter(altId -> altAlreadyInDb == altId).map(altId -> new DataNode(leaves: ["alternate-id": altId])).toList() }
             mockInventoryPersistenceService.getYangModelCmHandle(_) >> new YangModelCmHandle(alternateId: altAlreadyInDb)
         when: 'the batch of cm handle updates is checked'
             def result = objectUnderTest.getIdsOfCmHandlesWithRejectedAlternateId(batch, AlternateIdChecker.Operation.UPDATE)
@@ -75,7 +75,7 @@ class AlternateIdCheckerSpec extends Specification {
         given: 'a batch of 1 non-existing cm handle to update alternate id'
             def batch = [new NcmpServiceCmHandle(cmHandleId: 'non-existing', alternateId: 'altId')]
         and: 'the database does not contain any cm handles'
-            mockInventoryPersistenceService.getCmHandleDataNodeByAlternateId(_) >> { throwDataNodeNotFoundException() }
+            mockInventoryPersistenceService.getCmHandleDataNodesByAlternateIds(_) >> []
             mockInventoryPersistenceService.getYangModelCmHandle(_) >> { throwDataNodeNotFoundException() }
         when: 'the batch of cm handle updates is checked'
             def result = objectUnderTest.getIdsOfCmHandlesWithRejectedAlternateId(batch, AlternateIdChecker.Operation.UPDATE)

diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImplSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImplSpec.groovy
index e60bacb..fdf12a8 100644 (file)
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImplSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImplSpec.groovy
@@ -297,7 +297,7 @@ class InventoryPersistenceImplSpec extends Specification {
             1 * mockCpsDataService.getDataNodes(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, expectedXPath, INCLUDE_ALL_DESCENDANTS)
     }
 
-    def 'Get cm handle data node'() {
+    def 'Get cm handle data node by alternate id'() {
         given: 'expected xPath to get cmHandle data node'
             def expectedXPath = '/dmi-registry/cm-handles[@alternate-id=\'alternate id\']'
         and: 'query service is invoked with expected xpath'
@@ -316,6 +316,22 @@ class InventoryPersistenceImplSpec extends Specification {
             assert thrownException.getMessage().contains('DataNode not found')
     }
 
+    def 'Get multiple cm handle data nodes by alternate ids'() {
+        given: 'expected xPath to get cmHandle data node'
+            def expectedXPath = "/dmi-registry/cm-handles[@alternate-id='A' or @alternate-id='B']"
+        when: 'getting the cm handle data node'
+            objectUnderTest.getCmHandleDataNodesByAlternateIds(['A', 'B'])
+        then: 'query service is invoked with expected xpath'
+            1 * mockCmHandleQueries.queryNcmpRegistryByCpsPath(expectedXPath, OMIT_DESCENDANTS)
+    }
+
+    def 'Get multiple cm handle data nodes by alternate ids, passing empty collection'() {
+        when: 'getting the cm handle data node for no alternate ids'
+            objectUnderTest.getCmHandleDataNodesByAlternateIds([])
+        then: 'query service is not invoked'
+            0 * mockCmHandleQueries.queryNcmpRegistryByCpsPath(_, _)
+    }
+
     def 'Get CM handles that has given module names'() {
         when: 'the method to get cm handles is called'
             objectUnderTest.getCmHandleIdsWithGivenModules(['sample-module-name'])

diff --git a/docs/release-notes.rst b/docs/release-notes.rst
index 1652997..08d1ac4 100644 (file)
--- a/docs/release-notes.rst
+++ b/docs/release-notes.rst
@@ -46,6 +46,7 @@ Features
 --------
 3.5.2
     - `CPS-2326 <https://jira.onap.org/browse/CPS-2326>`_ Uplift liquibase-core dependency to 4.28.0
+    - `CPS-2366 <https://jira.onap.org/browse/CPS-2366>`_ Improve registration performance with use of alternateID
 
 Version: 3.5.1
 ==============