From 693cfdd2467d01bf9a90f588ba827454a7b20f33 Mon Sep 17 00:00:00 2001 From: ToineSiebelink Date: Tue, 9 Apr 2024 18:39:51 +0100 Subject: [PATCH] Introduce and handle Operation Too Large Exception for batch operations Issue-ID: CPS-2164 Signed-off-by: ToineSiebelink Change-Id: Iec05d2013be4f971309f0e75d84dc5d0936eb8ef --- cps-ncmp-rest/docs/openapi/components.yaml | 15 +++++++++-- cps-ncmp-rest/docs/openapi/ncmp.yml | 4 ++- .../NcmpPassthroughResourceRequestHandler.java | 17 +++++++++--- .../NetworkCmProxyRestExceptionHandler.java | 21 ++++++++------- .../rest/exceptions/PayloadTooLargeException.java | 31 ++++++++++++++++++++++ .../NcmpDatastoreRequestHandlerSpec.groovy | 23 +++++++++++----- .../NetworkCmProxyRestExceptionHandlerSpec.groovy | 26 +++++++++--------- 7 files changed, 103 insertions(+), 34 deletions(-) create mode 100644 cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/exceptions/PayloadTooLargeException.java diff --git a/cps-ncmp-rest/docs/openapi/components.yaml b/cps-ncmp-rest/docs/openapi/components.yaml index 0ad453a1c..8aa38c0f7 100644 --- a/cps-ncmp-rest/docs/openapi/components.yaml +++ b/cps-ncmp-rest/docs/openapi/components.yaml @@ -366,6 +366,7 @@ components: type: array items: type: string + description: targeted cm handles, maximum of 50 supported. If this limit is exceeded the request wil be refused. example: [ "da310eecdb8d44c2acc0ddaae01174b1","c748c58f8e0b438f9fd1f28370b17d47" ] examples: @@ -695,7 +696,7 @@ components: schema: $ref: '#/components/schemas/ErrorMessage' example: - status: 400 BAD_REQUEST + status: 400 message: Bad request error message details: Bad request error details Conflict: @@ -705,9 +706,19 @@ components: schema: $ref: '#/components/schemas/ErrorMessage' example: - status: 409 CONFLICT + status: 409 message: Conflict error message details: Conflict error details + PayloadTooLarge: + description: The request is larger than the server is willing or able to process + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorMessage' + example: + status: 413 + message: Payload Too Large error message + details: Payload Too Large error details NotImplemented: description: The given path has not been implemented content: diff --git a/cps-ncmp-rest/docs/openapi/ncmp.yml b/cps-ncmp-rest/docs/openapi/ncmp.yml index 0cb1cdffb..d0b1f35ea 100755 --- a/cps-ncmp-rest/docs/openapi/ncmp.yml +++ b/cps-ncmp-rest/docs/openapi/ncmp.yml @@ -194,7 +194,7 @@ dataOperationForCmHandle: tags: - network-cm-proxy summary: Execute a data operation for group of cm handle ids - description: This request will be handled asynchronously using messaging to the supplied topic. The rest response will be an acknowledge with a requestId to identify the relevant messages. + description: This request will be handled asynchronously using messaging to the supplied topic. The rest response will be an acknowledge with a requestId to identify the relevant messages. A maximum of 50 cm handles per operation is supported. operationId: executeDataOperationForCmHandles parameters: - $ref: 'components.yaml#/components/parameters/requiredTopicParamInQuery' @@ -216,6 +216,8 @@ dataOperationForCmHandle: $ref: 'components.yaml#/components/responses/BadRequest' 403: $ref: 'components.yaml#/components/responses/Forbidden' + 413: + $ref: 'components.yaml#/components/responses/PayloadTooLarge' 500: $ref: 'components.yaml#/components/responses/InternalServerError' 502: diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpPassthroughResourceRequestHandler.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpPassthroughResourceRequestHandler.java index 75112caf1..eca7ebfe3 100644 --- a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpPassthroughResourceRequestHandler.java +++ b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpPassthroughResourceRequestHandler.java @@ -33,6 +33,7 @@ import org.onap.cps.ncmp.api.impl.operations.OperationType; import org.onap.cps.ncmp.api.models.CmResourceAddress; import org.onap.cps.ncmp.api.models.DataOperationRequest; import org.onap.cps.ncmp.rest.exceptions.OperationNotSupportedException; +import org.onap.cps.ncmp.rest.exceptions.PayloadTooLargeException; import org.onap.cps.ncmp.rest.executor.CpsNcmpTaskExecutor; import org.onap.cps.ncmp.rest.util.TopicValidator; import org.springframework.http.ResponseEntity; @@ -45,6 +46,10 @@ public class NcmpPassthroughResourceRequestHandler extends NcmpDatastoreRequestH private static final Object noReturn = null; + private static final int MAXIMUM_CM_HANDLES_PER_OPERATION = 50; + + private static final String PAYLOAD_TOO_LARGE_TEMPLATE = "Operation '%s' affects too many (%d) cm handles"; + /** * Constructor. * @@ -101,17 +106,23 @@ public class NcmpPassthroughResourceRequestHandler extends NcmpDatastoreRequestH } private void validateDataOperationRequest(final String topicParamInQuery, - final DataOperationRequest - dataOperationRequest) { + final DataOperationRequest dataOperationRequest) { TopicValidator.validateTopicName(topicParamInQuery); dataOperationRequest.getDataOperationDefinitions().forEach(dataOperationDetail -> { if (OperationType.fromOperationName(dataOperationDetail.getOperation()) != READ) { throw new OperationNotSupportedException( dataOperationDetail.getOperation() + " operation not yet supported"); - } else if (DatastoreType.fromDatastoreName(dataOperationDetail.getDatastore()) == OPERATIONAL) { + } + if (DatastoreType.fromDatastoreName(dataOperationDetail.getDatastore()) == OPERATIONAL) { throw new InvalidDatastoreException(dataOperationDetail.getDatastore() + " datastore is not supported"); } + if (dataOperationDetail.getCmHandleIds().size() > MAXIMUM_CM_HANDLES_PER_OPERATION) { + final String errorMessage = String.format(PAYLOAD_TOO_LARGE_TEMPLATE, + dataOperationDetail.getOperationId(), + dataOperationDetail.getCmHandleIds().size()); + throw new PayloadTooLargeException(errorMessage); + } }); } 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 7498c5f6c..d32369191 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 @@ -1,7 +1,7 @@ /* * ============LICENSE_START======================================================= * Copyright (C) 2021 Pantheon.tech - * Modifications Copyright (C) 2021-2023 Nordix Foundation + * Modifications Copyright (C) 2021-2024 Nordix Foundation * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -60,8 +60,7 @@ public class NetworkCmProxyRestExceptionHandler { * @return response with response code 500. */ @ExceptionHandler - public static ResponseEntity handleInternalServerErrorExceptions( - final Exception exception) { + public static ResponseEntity handleInternalServerErrorExceptions(final Exception exception) { return buildErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR, exception); } @@ -73,7 +72,7 @@ public class NetworkCmProxyRestExceptionHandler { @ExceptionHandler({HttpClientRequestException.class}) public static ResponseEntity handleClientRequestExceptions( final HttpClientRequestException httpClientRequestException) { - return wrapDmiErrorResponse(HttpStatus.BAD_GATEWAY, httpClientRequestException); + return wrapDmiErrorResponse(httpClientRequestException); } @ExceptionHandler({DmiRequestException.class, DataValidationException.class, OperationNotSupportedException.class, @@ -88,10 +87,15 @@ public class NetworkCmProxyRestExceptionHandler { } @ExceptionHandler({DataNodeNotFoundException.class}) - public static ResponseEntity handleNotFoundExceptions(final CpsException exception) { + public static ResponseEntity handleNotFoundExceptions(final Exception exception) { return buildErrorResponse(HttpStatus.NOT_FOUND, exception); } + @ExceptionHandler({PayloadTooLargeException.class}) + public static ResponseEntity handlePayloadTooLargeExceptions(final Exception exception) { + return buildErrorResponse(HttpStatus.PAYLOAD_TOO_LARGE, exception); + } + private static ResponseEntity buildErrorResponse(final HttpStatus status, final Exception exception) { if (exception.getCause() != null || !(exception instanceof CpsException)) { log.error("Exception occurred", exception); @@ -111,15 +115,14 @@ public class NetworkCmProxyRestExceptionHandler { return new ResponseEntity<>(errorMessage, status); } - private static ResponseEntity wrapDmiErrorResponse( - final HttpStatus httpStatus, - final HttpClientRequestException httpClientRequestException) { + private static ResponseEntity wrapDmiErrorResponse(final HttpClientRequestException + httpClientRequestException) { final var dmiErrorMessage = new DmiErrorMessage(); final var dmiErrorResponse = new DmiErrorMessageDmiResponse(); dmiErrorResponse.setHttpCode(httpClientRequestException.getHttpStatus()); dmiErrorResponse.setBody(httpClientRequestException.getDetails()); dmiErrorMessage.setMessage(httpClientRequestException.getMessage()); dmiErrorMessage.setDmiResponse(dmiErrorResponse); - return new ResponseEntity<>(dmiErrorMessage, httpStatus); + return new ResponseEntity<>(dmiErrorMessage, HttpStatus.BAD_GATEWAY); } } diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/exceptions/PayloadTooLargeException.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/exceptions/PayloadTooLargeException.java new file mode 100644 index 000000000..cddbd0837 --- /dev/null +++ b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/exceptions/PayloadTooLargeException.java @@ -0,0 +1,31 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (C) 2024 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. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.cps.ncmp.rest.exceptions; + +public class PayloadTooLargeException extends RuntimeException { + + /** + * Instantiates a new payload too large exception. + */ + public PayloadTooLargeException(final String message) { + super(message); + } +} diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandlerSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandlerSpec.groovy index bdd0e716d..aef37c91e 100644 --- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandlerSpec.groovy +++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandlerSpec.groovy @@ -27,6 +27,7 @@ import org.onap.cps.ncmp.api.models.DataOperationDefinition import org.onap.cps.ncmp.api.models.DataOperationRequest import org.onap.cps.ncmp.api.models.CmResourceAddress import org.onap.cps.ncmp.rest.exceptions.OperationNotSupportedException +import org.onap.cps.ncmp.rest.exceptions.PayloadTooLargeException import org.onap.cps.ncmp.rest.executor.CpsNcmpTaskExecutor import spock.lang.Specification import spock.util.concurrent.PollingConditions @@ -110,9 +111,7 @@ class NcmpDatastoreRequestHandlerSpec extends Specification { } def 'Attempt to execute async data operation request with error #scenario'() { - given: 'notification feature is turned on' - objectUnderTest.notificationFeatureEnabled = true - and: 'a data operation definition with datastore: #datastore' + given: 'a data operation definition with datastore: #datastore' def dataOperationDefinition = new DataOperationDefinition(operation: 'read', datastore: datastore) when: 'data operation request is executed' def dataOperationRequest = new DataOperationRequest(dataOperationDefinitions: [dataOperationDefinition]) @@ -127,11 +126,9 @@ class NcmpDatastoreRequestHandlerSpec extends Specification { } def 'Attempt to execute async data operation request with #scenario operation: #operation.'() { - given: 'notification feature is turned on' - objectUnderTest.notificationFeatureEnabled = true - and: 'a data operation definition with operation: #operation' + given: 'a data operation definition with operation: #operation' def dataOperationDefinition = new DataOperationDefinition(operation: operation, datastore: 'ncmp-datastore:passthrough-running') - when: 'bulk request is executed' + when: 'data operation request is executed' objectUnderTest.executeRequest('someTopic', new DataOperationRequest(dataOperationDefinitions:[dataOperationDefinition]), NO_AUTH_HEADER) then: 'the expected type of exception is thrown' thrown(expectedException) @@ -144,4 +141,16 @@ class NcmpDatastoreRequestHandlerSpec extends Specification { 'unsupported' | 'delete' || OperationNotSupportedException } + def 'Attempt to execute async data operation request with too many cm handles.'() { + given: 'a data operation definition with too many cm handles' + def cmHandleIds = new String[51] + def dataOperationDefinition = new DataOperationDefinition(operationId: 'abc', operation: 'read', datastore: 'ncmp-datastore:passthrough-running', cmHandleIds: cmHandleIds) + when: 'data operation request is executed' + objectUnderTest.executeRequest('someTopic', new DataOperationRequest(dataOperationDefinitions:[dataOperationDefinition]), NO_AUTH_HEADER) + then: 'a payload too large exception is thrown' + def exceptionThrown = thrown(PayloadTooLargeException) + and: 'the error message contains the offending number of cm handles' + assert exceptionThrown.message == "Operation 'abc' affects too many (51) cm handles" + } + } 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 dd02b312a..a79ea25ab 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 @@ -1,7 +1,7 @@ /* * ============LICENSE_START======================================================= * Copyright (C) 2021 highstreet technologies GmbH - * Modifications Copyright (C) 2021-2023 Nordix Foundation + * Modifications Copyright (C) 2021-2024 Nordix Foundation * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,9 +23,10 @@ package org.onap.cps.ncmp.rest.exceptions import static org.springframework.http.HttpStatus.BAD_GATEWAY import static org.springframework.http.HttpStatus.BAD_REQUEST +import static org.springframework.http.HttpStatus.CONFLICT import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR import static org.springframework.http.HttpStatus.NOT_FOUND -import static org.springframework.http.HttpStatus.CONFLICT +import static org.springframework.http.HttpStatus.PAYLOAD_TOO_LARGE 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.test.web.servlet.request.MockMvcRequestBuilders.get @@ -111,22 +112,23 @@ class NetworkCmProxyRestExceptionHandlerSpec extends Specification { dataNodeBaseEndpointNcmpInventory = "$basePathNcmpInventory/v1" } - def 'Get request with generic #scenario exception returns correct HTTP Status with #scenario'() { + def 'Get request with #scenario exception returns correct HTTP Status with #scenario'() { when: 'an exception is thrown by the service' setupTestException(exception, NCMP) def response = performTestRequest(NCMP) then: 'an HTTP response is returned with correct message and details' assertTestResponse(response, expectedErrorCode, expectedErrorMessage, expectedErrorDetails) where: - scenario | exception || expectedErrorDetails | expectedErrorMessage | expectedErrorCode - 'CPS' | new CpsException(sampleErrorMessage, sampleErrorDetails) || sampleErrorDetails | sampleErrorMessage | INTERNAL_SERVER_ERROR - 'NCMP-server' | new ServerNcmpException(sampleErrorMessage, sampleErrorDetails) || null | sampleErrorMessage | INTERNAL_SERVER_ERROR - 'NCMP-client' | new DmiRequestException(sampleErrorMessage, sampleErrorDetails) || null | sampleErrorMessage | BAD_REQUEST - 'DataNode Validation' | new DataNodeNotFoundException('myDataspaceName', 'myAnchorName') || null | 'DataNode not found' | NOT_FOUND - 'other' | new IllegalStateException(sampleErrorMessage) || null | sampleErrorMessage | INTERNAL_SERVER_ERROR - 'Data Node Not Found' | new DataNodeNotFoundException('myDataspaceName', 'myAnchorName') || 'DataNode not found' | 'DataNode not found' | NOT_FOUND - 'Existing entry' | new AlreadyDefinedException('name',null) || 'name already exists' | 'Already defined exception' | CONFLICT - 'Existing entries' | AlreadyDefinedException.forDataNodes(['A', 'B'], 'myAnchorName') || '2 data node(s) already exist' | 'Already defined exception' | CONFLICT + scenario | exception || expectedErrorCode | expectedErrorMessage | expectedErrorDetails + 'CPS' | new CpsException(sampleErrorMessage, sampleErrorDetails) || INTERNAL_SERVER_ERROR | sampleErrorMessage | sampleErrorDetails + 'NCMP-server' | new ServerNcmpException(sampleErrorMessage, sampleErrorDetails) || INTERNAL_SERVER_ERROR | sampleErrorMessage | null + 'NCMP-client' | new DmiRequestException(sampleErrorMessage, sampleErrorDetails) || BAD_REQUEST | sampleErrorMessage | null + 'DataNode Validation' | new DataNodeNotFoundException('myDataspaceName', 'myAnchorName') || NOT_FOUND | 'DataNode not found' | null + 'other' | new IllegalStateException(sampleErrorMessage) || INTERNAL_SERVER_ERROR | sampleErrorMessage | null + 'Data Node Not Found' | new DataNodeNotFoundException('myDataspaceName', 'myAnchorName') || NOT_FOUND | 'DataNode not found' | 'DataNode not found' + 'Existing entry' | new AlreadyDefinedException('name',null) || CONFLICT | 'Already defined exception' | 'name already exists' + 'Existing entries' | AlreadyDefinedException.forDataNodes(['A', 'B'], 'myAnchorName') || CONFLICT | 'Already defined exception' | '2 data node(s) already exist' + 'Operation too large' | new PayloadTooLargeException(sampleErrorMessage) || PAYLOAD_TOO_LARGE | sampleErrorMessage | 'Check logs' } def 'Post request with exception returns correct HTTP Status.'() { -- 2.16.6