Improved code coverage for NetworkCmProxyDataServiceImpl

- Production code had catch block that is no longer needed
- Cleaned up some test somewhat (close to the new tests)

Issue-ID: CPS-475
Signed-off-by: ToineSiebelink <toine.siebelink@est.tech>
Change-Id: Iaa409e752a496bf088a58ccd422fe3d850442b59

3 files changed, 37 insertions, 41 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
index 6c1a1b464a..ea2f72fef7 100755
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
@@ -295,26 +295,17 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
      */
     public List<CmHandleRegistrationResponse> parseAndCreateCmHandlesInDmiRegistrationAndSyncModules(
             final DmiPluginRegistration dmiPluginRegistration) {
-        List<CmHandleRegistrationResponse> cmHandleRegistrationResponses = new ArrayList<>();
         final Map<YangModelCmHandle, CmHandleState> cmHandleStatePerCmHandle = new HashMap<>();
-        try {
-            dmiPluginRegistration.getCreatedCmHandles()
-                    .forEach(cmHandle -> {
-                        final YangModelCmHandle yangModelCmHandle = YangModelCmHandle.toYangModelCmHandle(
-                                dmiPluginRegistration.getDmiPlugin(),
-                                dmiPluginRegistration.getDmiDataPlugin(),
-                                dmiPluginRegistration.getDmiModelPlugin(),
-                                cmHandle);
-                        cmHandleStatePerCmHandle.put(yangModelCmHandle, CmHandleState.ADVISED);
-                    });
-            cmHandleRegistrationResponses = registerNewCmHandles(cmHandleStatePerCmHandle);
-        } catch (final DataValidationException dataValidationException) {
-            cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createFailureResponse(dmiPluginRegistration
-                            .getCreatedCmHandles().stream()
-                            .map(NcmpServiceCmHandle::getCmHandleId).findFirst().orElse(null),
-                    RegistrationError.CM_HANDLE_INVALID_ID));
-        }
-        return cmHandleRegistrationResponses;
+        dmiPluginRegistration.getCreatedCmHandles()
+                .forEach(cmHandle -> {
+                    final YangModelCmHandle yangModelCmHandle = YangModelCmHandle.toYangModelCmHandle(
+                            dmiPluginRegistration.getDmiPlugin(),
+                            dmiPluginRegistration.getDmiDataPlugin(),
+                            dmiPluginRegistration.getDmiModelPlugin(),
+                            cmHandle);
+                    cmHandleStatePerCmHandle.put(yangModelCmHandle, CmHandleState.ADVISED);
+                });
+        return registerNewCmHandles(cmHandleStatePerCmHandle);
     }
 
     protected List<CmHandleRegistrationResponse> parseAndRemoveCmHandlesInDmiRegistration(
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 c5436594a6..8942c42f13 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
@@ -288,9 +288,10 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
             1 * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch({ assert it.every { entry -> entry.value == CmHandleState.DELETING } })
         and: 'a response is received for all cm-handles'
             response.removedCmHandles.size() == 3
-        and: 'successfully de-registered cm handle entries are removed from in progress map'
+        and: 'successfully de-registered cm handle 1 is removed from in progress map'
             1 * mockModuleSyncStartedOnCmHandles.remove('cmhandle1')
-            1 * mockModuleSyncStartedOnCmHandles.remove('cmhandle3')
+        and: 'successfully de-registered cm handle 3 is removed from in progress map even though it was already being removed'
+            1 * mockModuleSyncStartedOnCmHandles.remove('cmhandle3') >> 'already in progress'
         and: 'failed de-registered cm handle entries should not be removed from in progress map'
             0 * mockModuleSyncStartedOnCmHandles.remove('cmhandle2')
         and: '1st and 3rd cm-handle deletes successfully'
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 af2b80f755..75af0439bd 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
@@ -54,6 +54,7 @@ import org.springframework.http.HttpStatus
 import org.springframework.http.ResponseEntity
 import spock.lang.Specification
 
+import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.OPERATIONAL
 import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.PASSTHROUGH_OPERATIONAL
 import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.PASSTHROUGH_RUNNING
 import static org.onap.cps.ncmp.api.impl.operations.OperationType.CREATE
@@ -108,31 +109,36 @@ class NetworkCmProxyDataServiceImplSpec extends Specification {
     }
 
     def 'Get resource data for pass-through operational from DMI.'() {
-        given: 'get data node is called'
+        given: 'cpsDataService returns valid data node'
             mockDataNode()
         and: 'get resource data from DMI is called'
-            mockDmiDataOperations.getResourceDataFromDmi(PASSTHROUGH_OPERATIONAL.datastoreName,'testCmHandle',
-                    'testResourceId', OPTIONS_PARAM, NO_TOPIC, NO_REQUEST_ID) >>
+            mockDmiDataOperations.getResourceDataFromDmi(PASSTHROUGH_OPERATIONAL.datastoreName,'testCmHandle', 'testResourceId', OPTIONS_PARAM, NO_TOPIC, NO_REQUEST_ID) >>
                     new ResponseEntity<>('dmi-response', HttpStatus.OK)
         when: 'get resource data operational for cm-handle is called'
-            def response = objectUnderTest.getResourceDataForCmHandle(PASSTHROUGH_OPERATIONAL.datastoreName, 'testCmHandle',
-                    'testResourceId', OPTIONS_PARAM, NO_TOPIC, NO_REQUEST_ID)
+            def response = objectUnderTest.getResourceDataForCmHandle(PASSTHROUGH_OPERATIONAL.datastoreName, 'testCmHandle', 'testResourceId', OPTIONS_PARAM, NO_TOPIC, NO_REQUEST_ID)
         then: 'DMI returns a json response'
-            response == 'dmi-response'
+            assert response == 'dmi-response'
     }
 
     def 'Get resource data for pass-through running from DMI.'() {
         given: 'cpsDataService returns valid data node'
             mockDataNode()
         and: 'DMI returns valid response and data'
-            mockDmiDataOperations.getResourceDataFromDmi(PASSTHROUGH_RUNNING.datastoreName, 'testCmHandle',
-                    'testResourceId', OPTIONS_PARAM, NO_TOPIC, NO_REQUEST_ID) >>
+            mockDmiDataOperations.getResourceDataFromDmi(PASSTHROUGH_RUNNING.datastoreName, 'testCmHandle', 'testResourceId', OPTIONS_PARAM, NO_TOPIC, NO_REQUEST_ID) >>
                     new ResponseEntity<>('{dmi-response}', HttpStatus.OK)
         when: 'get resource data is called'
-            def response = objectUnderTest.getResourceDataForCmHandle(PASSTHROUGH_RUNNING.datastoreName, 'testCmHandle',
-                    'testResourceId', OPTIONS_PARAM, NO_TOPIC, NO_REQUEST_ID)
+            def response = objectUnderTest.getResourceDataForCmHandle(PASSTHROUGH_RUNNING.datastoreName, 'testCmHandle', 'testResourceId', OPTIONS_PARAM, NO_TOPIC, NO_REQUEST_ID)
         then: 'get resource data returns expected response'
-            response == '{dmi-response}'
+            assert response == '{dmi-response}'
+    }
+
+    def 'Get resource data for operational (cached) data.'() {
+        given: 'CPS Data service returns some object(s)'
+            mockCpsDataService.getDataNodes(OPERATIONAL.datastoreName, 'testCmHandle', 'testResourceId', FetchDescendantsOption.OMIT_DESCENDANTS) >> ['First Object', 'other Object']
+        when: 'get resource data is called'
+            def response = objectUnderTest.getResourceDataForCmHandle(OPERATIONAL.datastoreName, 'testCmHandle', 'testResourceId', FetchDescendantsOption.OMIT_DESCENDANTS)
+        then: 'get resource data returns teh first object from the data service'
+            assert response == 'First Object'
     }
 
     def 'Execute (async) data operation for #datastoreName from DMI.'() {
@@ -243,7 +249,7 @@ class NetworkCmProxyDataServiceImplSpec extends Specification {
                 >> { new ResponseEntity<>(HttpStatus.OK) }
     }
 
-    def 'Verify modules and create anchor params'() {
+    def 'Verify modules and create anchor params.'() {
         given: 'dmi plugin registration return created cm handles'
             def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'service1', dmiModelPlugin: 'service1',
                     dmiDataPlugin: 'service2')
@@ -253,17 +259,15 @@ class NetworkCmProxyDataServiceImplSpec extends Specification {
             objectUnderTest.parseAndCreateCmHandlesInDmiRegistrationAndSyncModules(mockDmiPluginRegistration)
         then: 'system persists the cm handle state'
             1 * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(_) >> {
-                args ->
-                    {
-                        def cmHandleStatePerCmHandle = (args[0] as Map)
-                        cmHandleStatePerCmHandle.each {
-                            assert (it.key.id == 'test-cm-handle-id'
-                                    && it.value == CmHandleState.ADVISED)
-                        }
+                args -> {
+                          def cmHandleStatePerCmHandle = (args[0] as Map)
+                          cmHandleStatePerCmHandle.each {
+                            assert it.key.id == 'test-cm-handle-id' && it.value == CmHandleState.ADVISED
+                          }
                     }
             }
     }
-
+    
     def 'Execute cm handle id search'() {
         given: 'valid CmHandleQueryApiParameters input'
             def cmHandleQueryApiParameters = new CmHandleQueryApiParameters()