From: ToineSiebelink <toine.siebelink@est.tech>
Date: Tue, 5 Apr 2022 12:54:32 +0000 (+0100)
Subject: Improve branch coverage
X-Git-Tag: 1.2.0~18
X-Git-Url: https://gerrit.onap.org/r/gitweb?a=commitdiff_plain;h=54d10a74fdd8cfcb89fd1c0ab91abefdf0602367;p=cps%2Fncmp-dmi-plugin.git

Improve branch coverage

- update to oparent 3.3.0 to allow for checkstyle @SupressWarnings (not used in the end)
- refactor code to minimize unused branches (no more switch-statements :-))
- Added test for neccessary but uncovered branches

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

diff --git a/pom.xml b/pom.xml
index 2901fe05..616a8822 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1,6 +1,6 @@
 <!--
   ============LICENSE_START=======================================================
-  Copyright (c) 2021 Nordix Foundation.
+  Copyright (c) 2021-2022 Nordix Foundation.
   Modifications Copyright (C) 2021 Bell Canada.
   ================================================================================
   Licensed under the Apache License, Version 2.0 (the "License");
@@ -25,7 +25,7 @@
     <parent>
         <groupId>org.onap.oparent</groupId>
         <artifactId>oparent</artifactId>
-        <version>3.2.0</version>
+        <version>3.3.0</version>
         <relativePath/>
     </parent>
     <organization>
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 5544aeb3..653ebf7f 100644
--- 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
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021 Nordix Foundation
+ *  Copyright (C) 2021-2022 Nordix Foundation
  *  Modifications Copyright (C) 2022 Bell Canada
  *  ================================================================================
  *  Licensed under the Apache License, Version 2.0 (the "License");
@@ -21,10 +21,15 @@
 
 package org.onap.cps.ncmp.dmi.rest.controller;
 
+import static org.onap.cps.ncmp.dmi.model.DataAccessRequest.OperationEnum;
+
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import javax.validation.Valid;
+import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.onap.cps.ncmp.dmi.model.CmHandles;
 import org.onap.cps.ncmp.dmi.model.DataAccessRequest;
@@ -44,15 +49,22 @@ import org.springframework.web.bind.annotation.RestController;
 @RequestMapping("${rest.api.dmi-base-path}")
 @RestController
 @Slf4j
+@RequiredArgsConstructor
 public class DmiRestController implements DmiPluginApi, DmiPluginInternalApi {
 
-    private DmiService dmiService;
+    private final DmiService dmiService;
+
+    private final ObjectMapper objectMapper;
 
-    private ObjectMapper objectMapper;
+    private static final Map<OperationEnum, HttpStatus> operationToHttpStatusMap = new HashMap<>(6);
 
-    public DmiRestController(final DmiService dmiService, final ObjectMapper objectMapper) {
-        this.dmiService = dmiService;
-        this.objectMapper = objectMapper;
+    static {
+        operationToHttpStatusMap.put(null, HttpStatus.OK);
+        operationToHttpStatusMap.put(OperationEnum.READ, HttpStatus.OK);
+        operationToHttpStatusMap.put(OperationEnum.CREATE, HttpStatus.CREATED);
+        operationToHttpStatusMap.put(OperationEnum.PATCH, HttpStatus.OK);
+        operationToHttpStatusMap.put(OperationEnum.UPDATE, HttpStatus.OK);
+        operationToHttpStatusMap.put(OperationEnum.DELETE, HttpStatus.NO_CONTENT);
     }
 
     @Override
@@ -133,7 +145,7 @@ public class DmiRestController implements DmiPluginApi, DmiPluginInternalApi {
                 dataAccessRequest.getDataType(),
                 dataAccessRequest.getData());
         }
-        return new ResponseEntity<>(sdncResponse, getHttpStatus(dataAccessRequest));
+        return new ResponseEntity<>(sdncResponse, operationToHttpStatusMap.get(dataAccessRequest.getOperation()));
     }
 
     private boolean isReadOperation(final @Valid DataAccessRequest dataAccessRequest) {
@@ -141,30 +153,6 @@ public class DmiRestController implements DmiPluginApi, DmiPluginInternalApi {
             || dataAccessRequest.getOperation().equals(DataAccessRequest.OperationEnum.READ);
     }
 
-    private HttpStatus getHttpStatus(final DataAccessRequest dataAccessRequest) {
-        final HttpStatus httpStatus;
-        if (dataAccessRequest.getOperation() == null) {
-            httpStatus = HttpStatus.OK;
-        } else {
-            switch (dataAccessRequest.getOperation()) {
-                case CREATE:
-                    httpStatus = HttpStatus.CREATED;
-                    break;
-                case READ:
-                case UPDATE:
-                case PATCH:
-                    httpStatus = HttpStatus.OK;
-                    break;
-                case DELETE:
-                    httpStatus = HttpStatus.NO_CONTENT;
-                    break;
-                default:
-                    httpStatus = HttpStatus.BAD_REQUEST;
-            }
-        }
-        return httpStatus;
-    }
-
     private List<ModuleReference> convertRestObjectToJavaApiObject(
             final ModuleResourcesReadRequest moduleResourcesReadRequest) {
         return objectMapper
diff --git a/src/main/java/org/onap/cps/ncmp/dmi/service/operation/SdncOperations.java b/src/main/java/org/onap/cps/ncmp/dmi/service/operation/SdncOperations.java
index 7e2443e5..46a332cb 100644
--- a/src/main/java/org/onap/cps/ncmp/dmi/service/operation/SdncOperations.java
+++ b/src/main/java/org/onap/cps/ncmp/dmi/service/operation/SdncOperations.java
@@ -21,6 +21,8 @@
 
 package org.onap.cps.ncmp.dmi.service.operation;
 
+import static org.onap.cps.ncmp.dmi.model.DataAccessRequest.OperationEnum;
+
 import com.jayway.jsonpath.Configuration;
 import com.jayway.jsonpath.JsonPath;
 import com.jayway.jsonpath.JsonPathException;
@@ -30,6 +32,7 @@ import com.jayway.jsonpath.spi.mapper.JacksonMappingProvider;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
@@ -62,6 +65,16 @@ public class SdncOperations {
     private static final int QUERY_PARAM_VALUE_INDEX = 1;
     private static final int QUERY_PARAM_NAME_INDEX = 0;
 
+    private static Map<OperationEnum, HttpMethod> operationToHttpMethodMap = new HashMap<>(5);
+
+    static {
+        operationToHttpMethodMap.put(OperationEnum.READ, HttpMethod.GET);
+        operationToHttpMethodMap.put(OperationEnum.CREATE, HttpMethod.POST);
+        operationToHttpMethodMap.put(OperationEnum.PATCH, HttpMethod.PATCH);
+        operationToHttpMethodMap.put(OperationEnum.UPDATE, HttpMethod.PUT);
+        operationToHttpMethodMap.put(OperationEnum.DELETE, HttpMethod.DELETE);
+    }
+
     private final SdncProperties sdncProperties;
     private final SdncRestconfClient sdncRestconfClient;
     private final String topologyUrlData;
@@ -156,7 +169,7 @@ public class SdncOperations {
         final var getResourceDataUrl = prepareWriteUrl(nodeId, resourceId);
         final var httpHeaders = new HttpHeaders();
         httpHeaders.setContentType(MediaType.parseMediaType(contentType));
-        final HttpMethod httpMethod = getHttpMethod(operation);
+        final HttpMethod httpMethod = operationToHttpMethodMap.get(operation);
         return sdncRestconfClient.httpOperationWithJsonData(httpMethod, getResourceDataUrl, requestData, httpHeaders);
     }
 
@@ -230,30 +243,6 @@ public class SdncOperations {
         }
     }
 
-    private HttpMethod getHttpMethod(final DataAccessRequest.OperationEnum operation) {
-        HttpMethod httpMethod = null;
-        switch (operation) {
-            case READ:
-                httpMethod = HttpMethod.GET;
-                break;
-            case CREATE:
-                httpMethod = HttpMethod.POST;
-                break;
-            case PATCH:
-                httpMethod = HttpMethod.PATCH;
-                break;
-            case UPDATE:
-                httpMethod = HttpMethod.PUT;
-                break;
-            case DELETE:
-                httpMethod = HttpMethod.DELETE;
-                break;
-            default:
-                //unreachable code but checkstyle made me do this!
-        }
-        return httpMethod;
-    }
-
     private String getTopologyUrlData() {
         return UriComponentsBuilder.fromUriString(TOPOLOGY_URL_TEMPLATE_DATA)
                 .path("topology={topologyId}")
diff --git a/src/test/groovy/org/onap/cps/ncmp/dmi/rest/controller/ControllerSecuritySpec.groovy b/src/test/groovy/org/onap/cps/ncmp/dmi/rest/controller/ControllerSecuritySpec.groovy
index 14ab7147..8b13fc79 100644
--- a/src/test/groovy/org/onap/cps/ncmp/dmi/rest/controller/ControllerSecuritySpec.groovy
+++ b/src/test/groovy/org/onap/cps/ncmp/dmi/rest/controller/ControllerSecuritySpec.groovy
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021 Nordix Foundation
+ *  Copyright (C) 2021-2022 Nordix Foundation
  *  ================================================================================
  *  Licensed under the Apache License, Version 2.0 (the "License");
  *  you may not use this file except in compliance with the License.
@@ -20,6 +20,8 @@
 
 package org.onap.cps.ncmp.dmi.rest.controller
 
+import org.onap.cps.ncmp.dmi.config.WebSecurityConfig
+
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
 
 import org.springframework.beans.factory.annotation.Autowired
@@ -60,4 +62,13 @@ class ControllerSecuritySpec extends Specification {
         then: 'HTTP Unauthorized status code is returned'
             assert response.status == HttpStatus.UNAUTHORIZED.value()
     }
+
+    def 'Security Config #scenario permit URIs'() {
+        expect: 'can create a web security configuration'
+            new WebSecurityConfig(permitUris,'user','password')
+        where: 'the following string of permit URIs is provided'
+            scenario  | permitUris
+            'with'    | 'a,b'
+            'without' | ''
+    }
 }
diff --git a/src/test/groovy/org/onap/cps/ncmp/dmi/rest/controller/DmiRestControllerSpec.groovy b/src/test/groovy/org/onap/cps/ncmp/dmi/rest/controller/DmiRestControllerSpec.groovy
index b42eb4d2..2f200cfa 100644
--- a/src/test/groovy/org/onap/cps/ncmp/dmi/rest/controller/DmiRestControllerSpec.groovy
+++ b/src/test/groovy/org/onap/cps/ncmp/dmi/rest/controller/DmiRestControllerSpec.groovy
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2021 Nordix Foundation
+ *  Copyright (C) 2021-2022 Nordix Foundation
  *  Modifications Copyright (C) 2021-2022 Bell Canada
  *  ================================================================================
  *  Licensed under the Apache License, Version 2.0 (the "License");
@@ -21,7 +21,6 @@
 
 package org.onap.cps.ncmp.dmi.rest.controller
 
-import com.fasterxml.jackson.databind.ObjectMapper
 import org.onap.cps.ncmp.dmi.TestUtils
 import org.onap.cps.ncmp.dmi.exception.DmiException
 import org.onap.cps.ncmp.dmi.exception.ModuleResourceNotFoundException
@@ -36,7 +35,6 @@ import org.spockframework.spring.SpringBean
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.beans.factory.annotation.Value
 import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest
-import org.springframework.context.annotation.Import
 import org.springframework.http.HttpStatus
 import org.springframework.http.MediaType
 import org.springframework.security.test.context.support.WithMockUser
@@ -56,15 +54,14 @@ import static org.springframework.http.HttpStatus.OK
 
 @WebMvcTest(DmiRestController)
 @WithMockUser
-@Import(ObjectMapper)
 class DmiRestControllerSpec extends Specification {
 
-    @SpringBean
-    DmiService mockDmiService = Mock()
-
     @Autowired
     private MockMvc mvc
 
+    @SpringBean
+    DmiService mockDmiService = Mock()
+
     @Value('${rest.api.dmi-base-path}/v1')
     def basePathV1
 
@@ -95,18 +92,19 @@ class DmiRestControllerSpec extends Specification {
             def getModuleUrl = "$basePathV1/ch/node1/modules"
         and: 'given request body and get modules for cm-handle throws #exceptionClass'
             def json = '{"cmHandleProperties" : {}}'
-            mockDmiService.getModulesForCmHandle('node1') >> { throw Mock(exceptionClass) }
+            mockDmiService.getModulesForCmHandle('node1') >> { throw exception }
         when: 'post is invoked'
             def response = mvc.perform( post(getModuleUrl)
                     .contentType(MediaType.APPLICATION_JSON).content(json))
                     .andReturn().response
         then: 'response status is #expectedResponse'
-            response.status == expectedResponse
+            response.status == expectedResponse.value()
         where: 'the scenario is #scenario'
-            scenario                       |  exceptionClass                 || expectedResponse
-            'dmi service exception'        |  DmiException.class             || HttpStatus.INTERNAL_SERVER_ERROR.value()
-            'no modules found'             |  ModulesNotFoundException.class || HttpStatus.NOT_FOUND.value()
-            'any other runtime exception'  |  RuntimeException.class         || HttpStatus.INTERNAL_SERVER_ERROR.value()
+            scenario                       | exception                                        || expectedResponse
+            'dmi service exception'        | new DmiException('','')                          || HttpStatus.INTERNAL_SERVER_ERROR
+            'no modules found'             | new ModulesNotFoundException('','')              || HttpStatus.NOT_FOUND
+            'any other runtime exception'  | new RuntimeException()                           || HttpStatus.INTERNAL_SERVER_ERROR
+            'runtime exception with cause' | new RuntimeException('', new RuntimeException()) || HttpStatus.INTERNAL_SERVER_ERROR
     }
 
     def 'Register given list.'() {