Address minor code review comments for CPS-484

Issue-ID: CPS-565
Signed-off-by: niamhcore <niamh.core@est.tech>
Change-Id: I8cb28cf60622e5a9c0211b4fa7db2f255ede165a

diff --git a/docs/openapi/components.yml b/docs/openapi/components.yml
index ba2a0ec..bac5f9c 100644 (file)
--- a/docs/openapi/components.yml
+++ b/docs/openapi/components.yml
@@ -19,7 +19,7 @@ components:
           items:
             type: string
 
-    ModuleRequestParent:
+    DmiReadRequestBody:
       type: object
       properties:
         operation:

diff --git a/docs/openapi/openapi.yml b/docs/openapi/openapi.yml
index f169efd..5b9f7aa 100644 (file)
--- a/docs/openapi/openapi.yml
+++ b/docs/openapi/openapi.yml
@@ -107,7 +107,7 @@ paths:
         content:
           application/json:
             schema:
-              $ref: 'components.yml#/components/schemas/ModuleRequestParent'
+              $ref: 'components.yml#/components/schemas/DmiReadRequestBody'
       responses:
         '200':
           $ref: 'components.yml#/components/responses/Ok'

diff --git a/src/main/java/org/onap/cps/ncmp/dmi/rest/controller/DmiRestController.java b/src/main/java/org/onap/cps/ncmp/dmi/rest/controller/DmiRestController.java
index 32651e4..1d7abf2 100644 (file)
--- a/src/main/java/org/onap/cps/ncmp/dmi/rest/controller/DmiRestController.java
+++ b/src/main/java/org/onap/cps/ncmp/dmi/rest/controller/DmiRestController.java
@@ -29,8 +29,8 @@ import lombok.extern.slf4j.Slf4j;
 import org.onap.cps.ncmp.dmi.model.CmHandles;
 import org.onap.cps.ncmp.dmi.model.DataAccessReadRequest;
 import org.onap.cps.ncmp.dmi.model.DataAccessWriteRequest;
+import org.onap.cps.ncmp.dmi.model.DmiReadRequestBody;
 import org.onap.cps.ncmp.dmi.model.ModuleReference;
-import org.onap.cps.ncmp.dmi.model.ModuleRequestParent;
 import org.onap.cps.ncmp.dmi.model.ModuleSet;
 import org.onap.cps.ncmp.dmi.rest.api.DmiPluginApi;
 import org.onap.cps.ncmp.dmi.rest.api.DmiPluginInternalApi;
@@ -62,10 +62,10 @@ public class DmiRestController implements DmiPluginApi, DmiPluginInternalApi {
     }
 
     @Override
-    public ResponseEntity<Object> retrieveModuleResources(@Valid final ModuleRequestParent moduleRequestParent,
+    public ResponseEntity<Object> retrieveModuleResources(@Valid final DmiReadRequestBody dmiReadRequestBody,
         final String cmHandle) {
-        if (moduleRequestParent.getOperation().toString().equals("read")) {
-            final var moduleReferenceList = convertRestObjectToJavaApiObject(moduleRequestParent);
+        if (dmiReadRequestBody.getOperation().toString().equals("read")) {
+            final var moduleReferenceList = convertRestObjectToJavaApiObject(dmiReadRequestBody);
             final var response = dmiService.getModuleResources(cmHandle, moduleReferenceList);
             if (response.isEmpty()) {
                 return new ResponseEntity<>(response, HttpStatus.NOT_FOUND);
@@ -165,9 +165,9 @@ public class DmiRestController implements DmiPluginApi, DmiPluginInternalApi {
         return ResponseEntity.ok(modulesListAsJson);
     }
 
-    private List<ModuleReference> convertRestObjectToJavaApiObject(final ModuleRequestParent moduleRequestParent) {
+    private List<ModuleReference> convertRestObjectToJavaApiObject(final DmiReadRequestBody dmiReadRequestBody) {
         return objectMapper
-            .convertValue(moduleRequestParent.getData().getModules(), new TypeReference<List<ModuleReference>>() {
+            .convertValue(dmiReadRequestBody.getData().getModules(), new TypeReference<List<ModuleReference>>() {
             });
     }
 }
\ No newline at end of file

diff --git a/src/main/java/org/onap/cps/ncmp/dmi/service/DmiServiceImpl.java b/src/main/java/org/onap/cps/ncmp/dmi/service/DmiServiceImpl.java
index a139f7b..b4f0cac 100644 (file)
--- a/src/main/java/org/onap/cps/ncmp/dmi/service/DmiServiceImpl.java
+++ b/src/main/java/org/onap/cps/ncmp/dmi/service/DmiServiceImpl.java
@@ -100,10 +100,14 @@ public class DmiServiceImpl implements DmiService {
             final var responseEntity = sdncOperations.getModuleResource(cmHandle, moduleRequest);
             if (responseEntity.getStatusCode() == HttpStatus.OK) {
                 getModuleResponses.add(responseEntity.getBody());
-            } else {
+            } else if (responseEntity.getStatusCode() == HttpStatus.NOT_FOUND) {
                 log.error("SDNC did not return a module resource for the given cmHandle {}", cmHandle);
                 throw new ModuleResourceNotFoundException(cmHandle,
                     "SDNC did not return a module resource for the given cmHandle.");
+            } else {
+                log.error("Error occurred when getting module resources from SDNC for the given cmHandle {}", cmHandle);
+                throw new DmiException(cmHandle,
+                    "response code : " + responseEntity.getStatusCode() + " message : " + responseEntity.getBody());
             }
         }
         return getModuleResponses.toJSONString();

diff --git a/src/test/groovy/org/onap/cps/ncmp/dmi/service/DmiServiceImplSpec.groovy b/src/test/groovy/org/onap/cps/ncmp/dmi/service/DmiServiceImplSpec.groovy
index e370302..f793532 100644 (file)
--- a/src/test/groovy/org/onap/cps/ncmp/dmi/service/DmiServiceImplSpec.groovy
+++ b/src/test/groovy/org/onap/cps/ncmp/dmi/service/DmiServiceImplSpec.groovy
@@ -134,34 +134,47 @@ class DmiServiceImplSpec extends Specification {
             thrown(DmiException.class)
     }
 
-    def 'Get module resources.'() {
-        given: 'cmHandle, expected jsons and module reference list'
+    def 'Get a single module resource.'() {
+        given: 'a cmHandle and module reference list'
             def cmHandle = 'some-cmHandle'
-            def excpectedModulesJson1 = '{"ietf-netconf-monitoring:input":{"ietf-netconf-monitoring:identifier":"mRef1","ietf-netconf-monitoring:version":"mRefV1"}}'
-            def excpectedModulesJson2 = '{"ietf-netconf-monitoring:input":{"ietf-netconf-monitoring:identifier":"mRef2","ietf-netconf-monitoring:version":"mRefV2"}}'
-            def mRef1 = Mock(ModuleReference.class)
-            def mRef2 = Mock(ModuleReference.class)
-            mRef1.getName() >> 'mRef1'
-            mRef1.getRevision() >> 'mRefV1'
-            mRef2.getName() >> 'mRef2'
-            mRef2.getRevision() >> 'mRefV2'
-            def moduleList = [mRef1, mRef2] as LinkedList<ModuleReference>
+            def moduleReference = new ModuleReference(name: 'NAME',revision: 'REVISION')
+            def moduleList = [moduleReference]
+        and: 'the sdnc request body contains the correct name and revision'
+            def expectedRequestBody = '{"ietf-netconf-monitoring:input":{"ietf-netconf-monitoring:identifier":"NAME","ietf-netconf-monitoring:version":"REVISION"}}'
         when: 'get module resources is invoked with the given cm handle and a module list'
-            def response = objectUnderTest.getModuleResources(cmHandle, moduleList)
-        then: 'then get modules resources called correctly'
-            1 * mockSdncOperations.getModuleResource(cmHandle, excpectedModulesJson1) >> new ResponseEntity<String>('response-body1', HttpStatus.OK)
-            1 * mockSdncOperations.getModuleResource(cmHandle, excpectedModulesJson2) >> new ResponseEntity<String>('response-body2', HttpStatus.OK)
-        and: 'the response equals to the expected response body'
-            response == '["response-body1","response-body2"]'
+            def result = objectUnderTest.getModuleResources(cmHandle, moduleList)
+        then: 'get modules resources is called once with the expected cm handle and request body'
+            1 * mockSdncOperations.getModuleResource(cmHandle, expectedRequestBody) >> new ResponseEntity<String>('sdnc-response-body', HttpStatus.OK)
+        and: 'the result is a sdnc response wrapped inside an array'
+            assert result == '["sdnc-response-body"]'
     }
 
-    def 'Get module resources for a failed get module schema request.'() {
-        given: 'get module schema is invoked and returns not found'
-            mockSdncOperations.getModuleResource(_ as String, _ as String) >> new ResponseEntity<String>('some-response-body', HttpStatus.BAD_REQUEST)
+    def 'Get multiple module resources.'() {
+        given: 'a cmHandle and module reference list'
+            def cmHandle = 'some-cmHandle'
+            def moduleReference1 = new ModuleReference(name: 'name-1',revision: 'revision-1')
+            def moduleReference2 = new ModuleReference(name: 'name-2',revision: 'revision-2')
+            def moduleList = [moduleReference1, moduleReference2]
+        when: 'get module resources is invoked with the given cm handle and a module list'
+            def result = objectUnderTest.getModuleResources(cmHandle, moduleList)
+        then: 'get modules resources is called twice'
+            2 * mockSdncOperations.getModuleResource(cmHandle, _) >>> [new ResponseEntity<String>('sdnc-response-body1', HttpStatus.OK),
+                                                                   new ResponseEntity<String>('sdnc-response-body2', HttpStatus.OK)]
+        and: 'the response is the list of all module resource schemas returned by sdnc'
+            assert result == '["sdnc-response-body1","sdnc-response-body2"]'
+    }
+
+    def 'Get module resources when sdnc returns #scenario response.'() {
+        given: 'get module schema is invoked and returns a response from sdnc'
+            mockSdncOperations.getModuleResource(_ as String, _ as String) >> new ResponseEntity<String>('some-response-body', httpResponse)
         when: 'get module resources is invoked with the given cm handle and a module list'
             objectUnderTest.getModuleResources('some-cmHandle', [new ModuleReference()] as LinkedList<ModuleReference>)
         then: 'ModuleResourceNotFoundException is thrown'
-            thrown(ModuleResourceNotFoundException)
+            thrown(exception)
+        where: 'the following values are returned'
+            scenario            | httpResponse                     || exception
+            'not found'         | HttpStatus.NOT_FOUND             || ModuleResourceNotFoundException
+            'a internal server' | HttpStatus.INTERNAL_SERVER_ERROR || DmiException
     }
 
     def 'Get resource data for pass through operational from cm handle.'() {