diff options
author | ToineSiebelink <toine.siebelink@est.tech> | 2022-04-05 13:54:32 +0100 |
---|---|---|
committer | ToineSiebelink <toine.siebelink@est.tech> | 2022-04-06 10:10:56 +0100 |
commit | 54d10a74fdd8cfcb89fd1c0ab91abefdf0602367 (patch) | |
tree | 25e5d499b9074577db69a04d844186dc2424c639 | |
parent | 39450c583c097c3b083ca79a39c5f99a218f6cdc (diff) |
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
5 files changed, 58 insertions, 72 deletions
@@ -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.'() { |