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

5 files changed, 59 insertions, 24 deletions

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 f8e13b76d2..3600d6da32 100644
--- 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 beef752ef1..a0d3a3eaee 100644
--- 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
@@ -130,6 +130,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.
      *
      * @param cmHandleIds collection of cmHandle IDs
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 083b25db3d..42123f1a26 100644
--- 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;
@@ -184,6 +185,15 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv
     }
 
     @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());
         cmHandleIds.forEach(cmHandleId -> xpaths.add(getXPathForCmHandleById(cmHandleId)));
@@ -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 6642532ac2..b976ff4284 100644
--- 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 e60bacbdc5..fdf12a880d 100644
--- 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'])