From 80dee97e67b98b8308bedb1383dc624d7c642520 Mon Sep 17 00:00:00 2001 From: mpriyank Date: Wed, 23 Feb 2022 15:17:23 +0530 Subject: Fix success response code CM Handle Registration - changed registartion response code to 204 NO_CONTENT - improved exception handling for registration API - fixed failing csit related to changed response code - updated the release notes for CPS-892 and CPS-837 Issue-ID: CPS-892 Change-Id: I616e340debf1583b058e7ae6b8960972eec00f3e Signed-off-by: mpriyank --- cps-ncmp-rest/docs/openapi/ncmp-inventory.yml | 4 +- .../NetworkCmProxyInventoryController.java | 18 ++------ .../NetworkCmProxyRestExceptionHandler.java | 9 +++- .../NetworkCmProxyInventoryControllerSpec.groovy | 5 +- .../NetworkCmProxyRestExceptionHandlerSpec.groovy | 53 ++++++++++++++++++---- .../api/impl/NetworkCmProxyDataServiceImpl.java | 4 +- csit/tests/cps-model-sync/cps-model-sync.robot | 2 +- docs/api/swagger/ncmp/openapi-inventory.yaml | 4 +- docs/release-notes.rst | 2 + 9 files changed, 68 insertions(+), 33 deletions(-) diff --git a/cps-ncmp-rest/docs/openapi/ncmp-inventory.yml b/cps-ncmp-rest/docs/openapi/ncmp-inventory.yml index f3f84fed9a..3cd8e8baf2 100755 --- a/cps-ncmp-rest/docs/openapi/ncmp-inventory.yml +++ b/cps-ncmp-rest/docs/openapi/ncmp-inventory.yml @@ -31,8 +31,8 @@ updateDmiRegistration: schema: $ref: 'components.yaml#/components/schemas/RestDmiPluginRegistration' responses: - 201: - $ref: 'components.yaml#/components/responses/Created' + 204: + $ref: 'components.yaml#/components/responses/NoContent' 400: $ref: 'components.yaml#/components/responses/BadRequest' 401: diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/NetworkCmProxyInventoryController.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/NetworkCmProxyInventoryController.java index 3b72cec389..9e888fb1c5 100755 --- a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/NetworkCmProxyInventoryController.java +++ b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/NetworkCmProxyInventoryController.java @@ -1,12 +1,14 @@ /* * ============LICENSE_START======================================================= * Copyright (C) 2021 Bell Canada + * Modifications Copyright (C) 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. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -22,6 +24,7 @@ package org.onap.cps.ncmp.rest.controller; import com.fasterxml.jackson.databind.ObjectMapper; import javax.validation.Valid; +import lombok.RequiredArgsConstructor; import org.onap.cps.ncmp.api.NetworkCmProxyDataService; import org.onap.cps.ncmp.api.models.DmiPluginRegistration; import org.onap.cps.ncmp.rest.api.NetworkCmProxyInventoryApi; @@ -33,22 +36,12 @@ import org.springframework.web.bind.annotation.RestController; @RestController @RequestMapping("${rest.api.ncmp-inventory-base-path}") +@RequiredArgsConstructor public class NetworkCmProxyInventoryController implements NetworkCmProxyInventoryApi { private final NetworkCmProxyDataService networkCmProxyDataService; private final ObjectMapper objectMapper; - /** - * Constructor Injection for Dependencies. - * @param networkCmProxyDataService Data Service Interface - * @param objectMapper Object Mapper - */ - public NetworkCmProxyInventoryController(final NetworkCmProxyDataService networkCmProxyDataService, - final ObjectMapper objectMapper) { - this.networkCmProxyDataService = networkCmProxyDataService; - this.objectMapper = objectMapper; - } - /** * Update DMI Plugin Registration (used for first registration also). * @param restDmiPluginRegistration the registration data @@ -59,12 +52,11 @@ public class NetworkCmProxyInventoryController implements NetworkCmProxyInventor final DmiPluginRegistration dmiPluginRegistration = convertRestObjectToJavaApiObject(restDmiPluginRegistration); networkCmProxyDataService.updateDmiRegistrationAndSyncModule(dmiPluginRegistration); - return new ResponseEntity<>(HttpStatus.CREATED); + return new ResponseEntity<>(HttpStatus.NO_CONTENT); } private DmiPluginRegistration convertRestObjectToJavaApiObject( final RestDmiPluginRegistration restDmiPluginRegistration) { return objectMapper.convertValue(restDmiPluginRegistration, DmiPluginRegistration.class); } - } diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/exceptions/NetworkCmProxyRestExceptionHandler.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/exceptions/NetworkCmProxyRestExceptionHandler.java index d3450e596b..7dc6284fcc 100755 --- a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/exceptions/NetworkCmProxyRestExceptionHandler.java +++ b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/exceptions/NetworkCmProxyRestExceptionHandler.java @@ -27,8 +27,10 @@ import org.onap.cps.ncmp.api.impl.exception.DmiRequestException; import org.onap.cps.ncmp.api.impl.exception.NcmpException; import org.onap.cps.ncmp.api.impl.exception.ServerNcmpException; import org.onap.cps.ncmp.rest.controller.NetworkCmProxyController; +import org.onap.cps.ncmp.rest.controller.NetworkCmProxyInventoryController; import org.onap.cps.ncmp.rest.model.ErrorMessage; import org.onap.cps.spi.exceptions.CpsException; +import org.onap.cps.spi.exceptions.DataValidationException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ExceptionHandler; @@ -38,7 +40,7 @@ import org.springframework.web.bind.annotation.RestControllerAdvice; * Exception handler with error message return. */ @Slf4j -@RestControllerAdvice(assignableTypes = {NetworkCmProxyController.class}) +@RestControllerAdvice(assignableTypes = {NetworkCmProxyController.class, NetworkCmProxyInventoryController.class}) @NoArgsConstructor(access = AccessLevel.PACKAGE) public class NetworkCmProxyRestExceptionHandler { @@ -71,6 +73,11 @@ public class NetworkCmProxyRestExceptionHandler { return buildErrorResponse(HttpStatus.BAD_REQUEST, exception); } + @ExceptionHandler({DataValidationException.class}) + public static ResponseEntity handleDataValidatedExceptions(final DataValidationException exception) { + return buildErrorResponse(HttpStatus.BAD_REQUEST, exception); + } + private static ResponseEntity buildErrorResponse(final HttpStatus status, final Exception exception) { if (exception.getCause() != null || !(exception instanceof CpsException)) { log.error("Exception occurred", exception); diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyInventoryControllerSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyInventoryControllerSpec.groovy index 8d434e7758..3c603ed425 100644 --- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyInventoryControllerSpec.groovy +++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyInventoryControllerSpec.groovy @@ -8,6 +8,7 @@ * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -60,8 +61,8 @@ class NetworkCmProxyInventoryControllerSpec extends Specification { ).andReturn().response then: 'the cm handles are registered with the service' 1 * mockNetworkCmProxyDataService.updateDmiRegistrationAndSyncModule(_) - and: 'response status is created' - response.status == HttpStatus.CREATED.value() + and: 'response status is No Content' + response.status == HttpStatus.NO_CONTENT.value() } def 'Dmi plugin registration with #scenario' () { diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/exceptions/NetworkCmProxyRestExceptionHandlerSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/exceptions/NetworkCmProxyRestExceptionHandlerSpec.groovy index 1b72b8c084..35544d9411 100644 --- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/exceptions/NetworkCmProxyRestExceptionHandlerSpec.groovy +++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/exceptions/NetworkCmProxyRestExceptionHandlerSpec.groovy @@ -8,6 +8,7 @@ * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -22,22 +23,28 @@ package org.onap.cps.ncmp.rest.exceptions import groovy.json.JsonSlurper import org.modelmapper.ModelMapper +import org.onap.cps.TestUtils import org.onap.cps.ncmp.api.NetworkCmProxyDataService import org.onap.cps.ncmp.api.impl.exception.DmiRequestException import org.onap.cps.ncmp.api.impl.exception.ServerNcmpException import org.onap.cps.spi.exceptions.CpsException +import org.onap.cps.spi.exceptions.DataValidationException import org.onap.cps.utils.JsonObjectMapper 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.http.MediaType import org.springframework.test.web.servlet.MockMvc import spock.lang.Shared import spock.lang.Specification +import static org.onap.cps.ncmp.rest.exceptions.NetworkCmProxyRestExceptionHandlerSpec.ApiType.NCMP +import static org.onap.cps.ncmp.rest.exceptions.NetworkCmProxyRestExceptionHandlerSpec.ApiType.NCMPINVENTORY import static org.springframework.http.HttpStatus.BAD_REQUEST import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post @WebMvcTest class NetworkCmProxyRestExceptionHandlerSpec extends Specification { @@ -55,9 +62,13 @@ class NetworkCmProxyRestExceptionHandlerSpec extends Specification { JsonObjectMapper jsonObjectMapper = Stub() @Value('${rest.api.ncmp-base-path}') - def basePath + def basePathNcmp + + @Value('${rest.api.ncmp-inventory-base-path}') + def basePathNcmpInventory - def dataNodeBaseEndpoint + def dataNodeBaseEndpointNcmp + def dataNodeBaseEndpointNcmpInventory @Shared def errorMessage = 'some error message' @@ -65,13 +76,14 @@ class NetworkCmProxyRestExceptionHandlerSpec extends Specification { def errorDetails = 'some error details' def setup() { - dataNodeBaseEndpoint = "$basePath/v1" + dataNodeBaseEndpointNcmp = "$basePathNcmp/v1" + dataNodeBaseEndpointNcmpInventory = "$basePathNcmpInventory/v1" } def 'Get request with generic #scenario exception returns correct HTTP Status.'() { when: 'an exception is thrown by the service' - setupTestException(exception) - def response = performTestRequest() + setupTestException(exception, NCMP) + def response = performTestRequest(NCMP) then: 'an HTTP response is returned with correct message and details' assertTestResponse(response, expectedErrorCode, errorMessage, expectedErrorDetails) where: @@ -82,13 +94,29 @@ class NetworkCmProxyRestExceptionHandlerSpec extends Specification { 'other' | new IllegalStateException(errorMessage) || null | INTERNAL_SERVER_ERROR } - def setupTestException(exception){ - mockNetworkCmProxyDataService.getYangResourcesModuleReferences('testCmHandle')>> - { throw exception} + def 'Post request with exception returns correct HTTP Status.'() { + given: 'the service throws data validation exception' + def exception = new DataValidationException(errorMessage, errorDetails) + setupTestException(exception, NCMPINVENTORY) + when: 'the HTTP request is made' + def response = performTestRequest(NCMPINVENTORY) + then: 'an HTTP response is returned with correct message and details' + assertTestResponse(response, BAD_REQUEST, errorMessage, errorDetails) } - def performTestRequest(){ - return mvc.perform(get("$dataNodeBaseEndpoint/ch/testCmHandle/modules")).andReturn().response + def setupTestException(exception, apiType) { + if (NCMP.equals(apiType)) { + mockNetworkCmProxyDataService.getYangResourcesModuleReferences(*_) >> { throw exception } + } + mockNetworkCmProxyDataService.updateDmiRegistrationAndSyncModule(*_) >> { throw exception } + } + + def performTestRequest(apiType) { + if (NCMP.equals(apiType)) { + return mvc.perform(get("$dataNodeBaseEndpointNcmp/ch/testCmHandle/modules")).andReturn().response + } + def jsonData = TestUtils.getResourceFileContent('dmi-registration.json') + return mvc.perform(post("$dataNodeBaseEndpointNcmpInventory/ch").contentType(MediaType.APPLICATION_JSON).content(jsonData)).andReturn().response } static void assertTestResponse(response, expectedStatus , expectedErrorMessage , expectedErrorDetails) { @@ -98,4 +126,9 @@ class NetworkCmProxyRestExceptionHandlerSpec extends Specification { assert content['message'] == expectedErrorMessage assert expectedErrorDetails == null || content['details'] == expectedErrorDetails } + + enum ApiType { + NCMP, + NCMPINVENTORY; + } } 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 38f8e1707d..3c8d8bc3b7 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 @@ -92,8 +92,8 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService } } catch (final JsonProcessingException | DataNodeNotFoundException e) { final String errorMessage = String.format( - "Error occurred while processing the CM-handle registration request [%s] ,caused by : [%s]", - dmiPluginRegistration, e.getMessage()); + "Error occurred while processing the CM-handle registration request, caused by : [%s]", + e.getMessage()); throw new DataValidationException(errorMessage, e.getMessage(), e); } } diff --git a/csit/tests/cps-model-sync/cps-model-sync.robot b/csit/tests/cps-model-sync/cps-model-sync.robot index 0b9928bef8..99b9caa9a3 100644 --- a/csit/tests/cps-model-sync/cps-model-sync.robot +++ b/csit/tests/cps-model-sync/cps-model-sync.robot @@ -41,7 +41,7 @@ Register node, update data node and sync modules. ${uri}= Set Variable ${ncmpInventoryBasePath}/v1/ch ${headers}= Create Dictionary Content-Type=application/json Authorization=${auth} ${response}= POST On Session CPS_URL ${uri} headers=${headers} data=${jsonData} - Should Be Equal As Strings ${response.status_code} 201 + Should Be Equal As Strings ${response.status_code} 204 Get modules for registered data node ${uri}= Set Variable ${ncmpBasePath}/v1/ch/PNFDemo/modules diff --git a/docs/api/swagger/ncmp/openapi-inventory.yaml b/docs/api/swagger/ncmp/openapi-inventory.yaml index 67eae41c46..34a087b85c 100644 --- a/docs/api/swagger/ncmp/openapi-inventory.yaml +++ b/docs/api/swagger/ncmp/openapi-inventory.yaml @@ -20,8 +20,8 @@ paths: $ref: '#/components/schemas/RestDmiPluginRegistration' required: true responses: - "201": - description: Created + "204": + description: No Content content: {} "400": description: Bad Request diff --git a/docs/release-notes.rst b/docs/release-notes.rst index c7d5d39e6a..52d44ac4c2 100755 --- a/docs/release-notes.rst +++ b/docs/release-notes.rst @@ -32,6 +32,7 @@ Features - `CPS-741 `_ Re sync after removing cm handles - `CPS-777 `_ Ensure all DMI operations use POST method - `CPS-780 `_ Add examples for parameters, request and response in openapi yaml for cps-core + - `CPS-837 `_ Add Remove and Update properties (DMI and Public) as part of CM Handle Registration update Bug Fixes --------- @@ -42,6 +43,7 @@ Bug Fixes - `CPS-841 `_ Upgrade log4j to 2.17.1 as recommended by ONAP SECCOM - `CPS-867 `_ Database port made configurable through env variable DB_PORT - `CPS-856 `_ Retry mechanism not working for concurrent CmHandle registration + - `CPS-892 `_ Fixed the response code during CM-Handle Registration from 201 CREATED to 204 NO_CONTENT Known Limitations, Issues and Workarounds ----------------------------------------- -- cgit 1.2.3-korg