Merge "Error reporting when registering cm handle with alternate id 2 - update scenario"

4 files changed, 72 insertions, 59 deletions

diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy
index cb7e1ef8a..fbfbac59e 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy
@@ -78,7 +78,7 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
 
     def setup() {
         // always accept all cm handles
-        mockAlternateIdChecker.getIdsOfCmHandlesWithRejectedAlternateId(_) >> []
+        mockAlternateIdChecker.getIdsOfCmHandlesWithRejectedAlternateId(*_) >> []
 
         // always can find all cm handles in DB
         mockInventoryPersistence.getYangModelCmHandles(_) >> { args -> args[0].collect { new YangModelCmHandle(id:it) } }
@@ -418,14 +418,4 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
         'an unexpected exception'    | 'cmhandle'             | new RuntimeException('Failed')            || UNKNOWN_ERROR        | 'Failed'
     }
 
-    def 'Adding data to alternate id caches.'() {
-        given: 'a registration with three CM Handles to be created'
-            def ncmpServiceCmHandles = [new NcmpServiceCmHandle(cmHandleId: 'cmhandle1', alternateId: 'my-alternate-id-1')]
-            def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server', createdCmHandles: ncmpServiceCmHandles)
-        when: 'the DMI plugin registration happens'
-            objectUnderTest.updateDmiRegistrationAndSyncModule(dmiPluginRegistration)
-        then: 'the new alternate id is added to the cache'
-            1 * mockAlternateIdChecker.getIdsOfCmHandlesWithRejectedAlternateId(ncmpServiceCmHandles) >> ['cmhandle1']
-    }
-
 }
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy
index 5e837ac6b..74016e4c0 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy
@@ -274,7 +274,7 @@ class NetworkCmProxyDataServiceImplSpec extends Specification {
             dmiPluginRegistration.createdCmHandles = [ncmpServiceCmHandle]
             mockDmiPluginRegistration.getCreatedCmHandles() >> [ncmpServiceCmHandle]
         and: 'no rejected cm handles because of alternate ids'
-            mockAlternateIdChecker.getIdsOfCmHandlesWithRejectedAlternateId(_) >> []
+            mockAlternateIdChecker.getIdsOfCmHandlesWithRejectedAlternateId(*_) >> []
         when: 'parse and create cm handle in dmi registration then sync module'
             mockDmiPluginRegistration.createdCmHandles = ['test-cm-handle-id']
             objectUnderTest.processCreatedCmHandles(mockDmiPluginRegistration, new DmiPluginRegistrationResponse())
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy
index e00a42674..cbed4177b 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy
@@ -49,6 +49,12 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification {
     def mockAlternateIdChecker = Mock(AlternateIdChecker)
 
     def objectUnderTest = new NetworkCmProxyDataServicePropertyHandler(mockInventoryPersistence, mockCpsDataService, jsonObjectMapper, mockAlternateIdChecker)
+
+    def setup() {
+        // Always accept all alternate IDs
+        mockAlternateIdChecker.getIdsOfCmHandlesWithRejectedAlternateId(*_) >> []
+    }
+
     def static cmHandleId = 'myHandle1'
     def static cmHandleXpath = "/dmi-registry/cm-handles[@id='${cmHandleId}']"
 
@@ -66,7 +72,7 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification {
         when: 'update data node leaves is called with the update request'
             objectUnderTest.updateCmHandleProperties(cmHandleUpdateRequest)
         then: 'the replace list method is called with correct params'
-            1 * mockInventoryPersistence.replaceListContent(cmHandleXpath,_) >> { args ->
+            1 * mockInventoryPersistence.replaceListContent(cmHandleXpath, _) >> { args ->
                 {
                     assert args[1].leaves.size() == expectedPropertiesAfterUpdate.size()
                     assert args[1].leaves.containsAll(convertToProperties(expectedPropertiesAfterUpdate))
@@ -142,10 +148,10 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification {
                 assert it.errorText == expectedErrorText
             }
         where:
-        scenario                   | cmHandleId               | exception                                                                                           || expectedError        | expectedErrorText
-        'Cm Handle does not exist' | 'cmHandleId'             | new DataNodeNotFoundException(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR)                        || CM_HANDLES_NOT_FOUND | 'cm handle id(s) not found'
-        'Unknown'                  | 'cmHandleId'             | new RuntimeException('Failed')                                                                      || UNKNOWN_ERROR        | 'Failed'
-        'Invalid cm handle id'     | 'cmHandleId with spaces' | new DataValidationException('Name Validation Error.', cmHandleId + 'contains an invalid character') || CM_HANDLE_INVALID_ID | 'cm-handle has an invalid character(s) in id'
+            scenario                   | cmHandleId               | exception                                                                                           || expectedError        | expectedErrorText
+            'Cm Handle does not exist' | 'cmHandleId'             | new DataNodeNotFoundException(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR)                        || CM_HANDLES_NOT_FOUND | 'cm handle id(s) not found'
+            'Unknown'                  | 'cmHandleId'             | new RuntimeException('Failed')                                                                      || UNKNOWN_ERROR        | 'Failed'
+            'Invalid cm handle id'     | 'cmHandleId with spaces' | new DataValidationException('Name Validation Error.', cmHandleId + 'contains an invalid character') || CM_HANDLE_INVALID_ID | 'cm-handle has an invalid character(s) in id'
     }
 
     def 'Multiple update operations in a single request'() {
@@ -177,42 +183,44 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification {
                 assert it.errorText == 'cm handle id(s) not found'
             }
         then: 'the replace list method is called twice'
-            2 * mockInventoryPersistence.replaceListContent(cmHandleXpath,_)
+            2 * mockInventoryPersistence.replaceListContent(cmHandleXpath, _)
     }
 
-    def 'Update CM Handle Alternate ID with #scenario'() {
-        given: 'an existing cm handle'
-            DataNode existingCmHandleDataNode = new DataNode(xpath: cmHandleXpath, leaves: ['id': cmHandleId])
-        and: 'an update request with an alternate id'
-            def ncmpServiceCmHandle = new NcmpServiceCmHandle(cmHandleId: cmHandleId, alternateId: 'alt-1')
-        when: 'update alternate id method is called with the update request'
-            objectUnderTest.updateAlternateId(existingCmHandleDataNode, ncmpServiceCmHandle)
-        then: 'the update node leaves method is invoked as many times as expected'
-            callsToDataService * mockCpsDataService.updateNodeLeaves('NCMP-Admin', 'ncmp-dmi-registry', '/dmi-registry', _, _) >>
+    def 'Update alternate id of existing CM Handle.'() {
+        given: 'cm handles request'
+            def cmHandleUpdateRequest = [new NcmpServiceCmHandle(cmHandleId: cmHandleId, alternateId: 'alt-1')]
+        and: 'a data node found'
+            def dataNode = new DataNode(xpath: cmHandleXpath, leaves: ['id': cmHandleId, 'alternate-id': 'alt-1'])
+            mockInventoryPersistence.getCmHandleDataNodeByCmHandleId(cmHandleId) >> [dataNode]
+        when: 'cm handle properties is updated'
+            def response = objectUnderTest.updateCmHandleProperties(cmHandleUpdateRequest)
+        then: 'the update is delegated to cps data service with correct parameters'
+            1 * mockCpsDataService.updateNodeLeaves('NCMP-Admin', 'ncmp-dmi-registry', '/dmi-registry', _, _) >>
                     { args ->
                         assert args[3].contains('alt-1')
                     }
-            mockAlternateIdChecker.canApplyAlternateId(cmHandleId, '','alt-1') >> isNewMapping
-        where: 'following updates are attempted'
-            scenario                | isNewMapping || callsToDataService
-            'new alternate id   '   | true         || 1
-            'existing alternate id' | false        || 0
+        and: 'one successful registration response'
+            response.size() == 1
+        and: 'the response shows success for the given cm handle id'
+                assert response[0].status == Status.SUCCESS
+                assert response[0].cmHandle == cmHandleId
     }
 
-    def 'Alternate ID removed from cache when persisting fails.'() {
-        given: 'an existing data node and an update request with an alternate id'
-            def ncmpServiceCmHandle = new NcmpServiceCmHandle(cmHandleId: cmHandleId, alternateId: 'alt-1')
-            DataNode existingCmHandleDataNode = new DataNode(xpath: cmHandleXpath, leaves: ['id': cmHandleId, 'alternate-id': null])
-        and: 'an applicable alternate id for the cm handle'
-            mockAlternateIdChecker.canApplyAlternateId(cmHandleId, '','alt-1') >> true
-        and: 'but an exception occurs while saving'
-            def originalException = new NullPointerException('some exception')
-            mockCpsDataService.updateNodeLeaves(*_) >> { throw originalException }
-        when: 'updating of alternate id called'
-            objectUnderTest.updateAlternateId(existingCmHandleDataNode, ncmpServiceCmHandle)
-        then: 'the original exception is thrown up'
-            def thrownException = thrown(NullPointerException)
-            assert thrownException == originalException
+    def 'Update with rejected alternate id.'() {
+        given: 'cm handles request'
+            def updatedNcmpServiceCmHandles = [new NcmpServiceCmHandle(cmHandleId: cmHandleId, alternateId: 'alt-1')]
+        and: 'a data node found'
+            def dataNode = new DataNode(xpath: cmHandleXpath, leaves: ['id': cmHandleId, 'alternate-id': 'alt-1'])
+            mockInventoryPersistence.getCmHandleDataNodeByCmHandleId(cmHandleId) >> [dataNode]
+        when: 'attempt to update the cm handle'
+            def response = objectUnderTest.updateCmHandleProperties(updatedNcmpServiceCmHandles)
+        then: 'the update is NOT delegated to cps data service'
+            0 * mockCpsDataService.updateNodeLeaves(*_)
+        and:  'the alternate id checker rejects the given cm handle (override default setup behavior)'
+            mockAlternateIdChecker.getIdsOfCmHandlesWithRejectedAlternateId(*_) >> [cmHandleId]
+        and: 'the response shows a failure for the given cm handle id'
+            assert response[0].status == Status.FAILURE
+            assert response[0].cmHandle == cmHandleId
     }
 
     def convertToProperties(expectedPropertiesAfterUpdateAsMap) {
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/AlternateIdCheckerSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/AlternateIdCheckerSpec.groovy
index aaa034437..0eabaa1d2 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/AlternateIdCheckerSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/AlternateIdCheckerSpec.groovy
@@ -20,7 +20,6 @@
 
 package org.onap.cps.ncmp.api.impl.utils
 
-
 import org.onap.cps.ncmp.api.impl.inventory.InventoryPersistence
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
 import org.onap.cps.ncmp.api.models.NcmpServiceCmHandle
@@ -68,25 +67,41 @@ class AlternateIdCheckerSpec extends Specification {
             'other alternate id' || false
     }
 
-    def 'Check a batch of NEW cm handles with #scenario.'() {
+    def 'Check a batch of created cm handles with #scenario.'() {
         given: 'a batch of 2 new cm handles alternate id ids #alt1 and #alt2'
             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: #alreadyinDb'
+        and: 'the database already contains cm handle(s) with these alternate ids: #altAlreadyInDb'
             mockInventoryPersistenceService.getCmHandleDataNodeByAlternateId(_) >>
                 {  args -> altAlreadyInDb.contains(args[0]) ? new DataNode() : throwDataNodeNotFoundException() }
         when: 'the batch of new cm handles is checked'
-            def result = objectUnderTest.getIdsOfCmHandlesWithRejectedAlternateId(batch)
-        then: 'the result only contains the ids of the acceptable cm handles'
-            assert result.contains('ch-1') == rejectCh1
-            assert result.contains('ch-2') == rejectCh2
+            def result = objectUnderTest.getIdsOfCmHandlesWithRejectedAlternateId(batch, AlternateIdChecker.Operation.CREATE)
+        then: 'the result contains ids of the rejected cm handles'
+            assert result == expectedRejectedCmHandleIds
         where: 'the following alternate ids are used'
-            scenario                          | alt1   | alt2   | altAlreadyInDb  || rejectCh1 | rejectCh2
-            'no alternate ids'                | ''     | ''     | ['dont matter'] || false      | false
-            'new alternate ids'               | 'fdn1' | 'fdn2' | ['other fdn']   || false      | false
-            'one already used alternate id'   | 'fdn1' | 'fdn2' | ['fdn1']        || true       | false
-            'two already used alternate ids'  | 'fdn1' | 'fdn2' | ['fdn1','fdn2'] || true       | true
-            'duplicate alternate id in batch' | 'fdn1' | 'fdn1' | ['dont matter'] || false      | true
+            scenario                          | alt1   | alt2   | altAlreadyInDb  || expectedRejectedCmHandleIds
+            'no alternate ids'                | ''     | ''     | ['dont matter'] || []
+            'new alternate ids'               | 'fdn1' | 'fdn2' | ['other fdn']   || []
+            'one already used alternate id'   | 'fdn1' | 'fdn2' | ['fdn1']        || ['ch-1']
+            'duplicate alternate id in batch' | 'fdn1' | 'fdn1' | ['dont matter'] || ['ch-2']
+    }
+
+    def 'Check a batch of updates to existing cm handles with #scenario.'() {
+        given: 'a batch of 1 existing cm handle 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.getYangModelCmHandle(_) >> new YangModelCmHandle(alternateId: altAlreadyInDb)
+        when: 'the batch of cm handle updates is checked'
+            def result = objectUnderTest.getIdsOfCmHandlesWithRejectedAlternateId(batch, AlternateIdChecker.Operation.UPDATE)
+        then: 'the result contains ids of the rejected cm handles'
+            assert result == expectedRejectedCmHandleIds
+        where: 'the following parameters are used'
+            scenario                      | proposedAlt | altAlreadyInDb || expectedRejectedCmHandleIds
+            'no alternate id'             | 'fdn1'      | ''             || []
+            'used the same alternate id'  | 'fdn1'      | 'fdn1'         || []
+            'used different alternate id' | 'otherFdn'  | 'fdn1'         || ['ch-1']
     }
 
     def throwDataNodeNotFoundException() {