From 5fd49caaec2ec7ab5424dc0f26209d437c7387cd Mon Sep 17 00:00:00 2001 From: ToineSiebelink Date: Tue, 18 Jul 2023 13:23:29 +0100 Subject: Refactor ncmp request handlers (fix async issue) - simplified request handlers (sub)classes (no more need for default interface) - fix issue with async execution of data operation requests (wasn't really async) - removed redundant (unreachable) check in production code - Improved code coverage (branches) ncmp request handlers added UNIT test - removed MVC test scenarios now covered by appropriate unit level tests - applied Spock Polling Condition for verifying async call Issue-ID: CPS-475 Signed-off-by: ToineSiebelink Change-Id: Ibe601c709de65080fa8898f2419fdbd92c5ba27d --- .../handlers/NcmpCachedResourceRequestHandler.java | 61 +++++++++------ .../handlers/NcmpDatastoreRequestHandler.java | 88 ++++----------------- .../NcmpPassthroughResourceRequestHandler.java | 90 ++++++++++++++++++---- .../handlers/TaskManagementDefaultHandler.java | 59 -------------- .../ncmp/rest/executor/CpsNcmpTaskExecutor.java | 10 +-- 5 files changed, 131 insertions(+), 177 deletions(-) delete mode 100644 cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/TaskManagementDefaultHandler.java (limited to 'cps-ncmp-rest/src/main/java/org/onap') diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpCachedResourceRequestHandler.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpCachedResourceRequestHandler.java index 76946d3af..85a1eae23 100644 --- a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpCachedResourceRequestHandler.java +++ b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpCachedResourceRequestHandler.java @@ -25,6 +25,7 @@ import org.onap.cps.ncmp.api.NetworkCmProxyDataService; import org.onap.cps.ncmp.api.NetworkCmProxyQueryService; import org.onap.cps.ncmp.rest.executor.CpsNcmpTaskExecutor; import org.onap.cps.spi.FetchDescendantsOption; +import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Component; @Component @@ -48,37 +49,53 @@ public class NcmpCachedResourceRequestHandler extends NcmpDatastoreRequestHandle this.networkCmProxyQueryService = networkCmProxyQueryService; } + /** + * Executes a synchronous query request for given cm handle. + * Note. Currently only ncmp-datastore:operational supports query operations. + * + * @param cmHandleId the cm handle + * @param resourceIdentifier the resource identifier + * @param includeDescendants whether include descendants + * @return the response entity + */ + public ResponseEntity executeRequest(final String cmHandleId, + final String resourceIdentifier, + final boolean includeDescendants) { + + final Supplier taskSupplier = getTaskSupplierForQueryRequest(cmHandleId, resourceIdentifier, + includeDescendants); + return executeTaskSync(taskSupplier); + } + @Override - public Supplier getTaskSupplierForGetRequest(final String datastoreName, - final String cmHandleId, - final String resourceIdentifier, - final String optionsParamInQuery, - final String topicParamInQuery, - final String requestId, - final boolean includeDescendants) { + protected Supplier getTaskSupplierForGetRequest(final String datastoreName, + final String cmHandleId, + final String resourceIdentifier, + final String optionsParamInQuery, + final String topicParamInQuery, + final String requestId, + final boolean includeDescendants) { - final FetchDescendantsOption fetchDescendantsOption = - TaskManagementDefaultHandler.getFetchDescendantsOption(includeDescendants); + final FetchDescendantsOption fetchDescendantsOption = getFetchDescendantsOption(includeDescendants); return () -> networkCmProxyDataService.getResourceDataForCmHandle(datastoreName, cmHandleId, resourceIdentifier, - fetchDescendantsOption); + fetchDescendantsOption); } - /** - * Gets ncmp datastore query handler. - * Note. Currently only ncmp-datastore:operational supports query operations - * @return a ncmp datastore query handler. - */ - @Override - public Supplier getTaskSupplierForQueryRequest(final String cmHandleId, - final String resourceIdentifier, - final boolean includeDescendants) { + private Supplier getTaskSupplierForQueryRequest(final String cmHandleId, + final String resourceIdentifier, + final boolean includeDescendants) { - final FetchDescendantsOption fetchDescendantsOption = - TaskManagementDefaultHandler.getFetchDescendantsOption(includeDescendants); + final FetchDescendantsOption fetchDescendantsOption = getFetchDescendantsOption(includeDescendants); return () -> networkCmProxyQueryService.queryResourceDataOperational(cmHandleId, resourceIdentifier, - fetchDescendantsOption); + fetchDescendantsOption); } + private static FetchDescendantsOption getFetchDescendantsOption(final boolean includeDescendants) { + return includeDescendants ? FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS + : FetchDescendantsOption.OMIT_DESCENDANTS; + } + + } diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandler.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandler.java index d7aeab6b0..d40ab9b39 100644 --- a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandler.java +++ b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandler.java @@ -20,30 +20,24 @@ package org.onap.cps.ncmp.rest.controller.handlers; -import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.OPERATIONAL; -import static org.onap.cps.ncmp.api.impl.operations.OperationType.READ; - import java.util.Map; import java.util.UUID; import java.util.function.Supplier; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import org.onap.cps.ncmp.api.impl.exception.InvalidDatastoreException; -import org.onap.cps.ncmp.api.impl.operations.DatastoreType; -import org.onap.cps.ncmp.api.impl.operations.OperationType; -import org.onap.cps.ncmp.api.models.DataOperationRequest; -import org.onap.cps.ncmp.rest.exceptions.OperationNotSupportedException; import org.onap.cps.ncmp.rest.executor.CpsNcmpTaskExecutor; import org.onap.cps.ncmp.rest.util.TopicValidator; import org.springframework.beans.factory.annotation.Value; -import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; @Slf4j @Service @RequiredArgsConstructor -public class NcmpDatastoreRequestHandler implements TaskManagementDefaultHandler { +public abstract class NcmpDatastoreRequestHandler { + + private static final String NO_REQUEST_ID = null; + private static final String NO_TOPIC = null; @Value("${notification.async.executor.time-out-value-in-ms:2000}") protected int timeOutInMilliSeconds; @@ -51,10 +45,10 @@ public class NcmpDatastoreRequestHandler implements TaskManagementDefaultHandler @Value("${notification.enabled:true}") protected boolean notificationFeatureEnabled; - private final CpsNcmpTaskExecutor cpsNcmpTaskExecutor; + protected final CpsNcmpTaskExecutor cpsNcmpTaskExecutor; /** - * Executes synchronous/asynchronous request for given cm handle. + * Executes synchronous/asynchronous get request for given cm handle. * * @param datastoreName the name of the datastore * @param cmHandleId the cm handle @@ -86,46 +80,10 @@ public class NcmpDatastoreRequestHandler implements TaskManagementDefaultHandler return executeTaskSync(taskSupplier); } - /** - * Executes a synchronous request for given cm handle. - * Note. Currently only ncmp-datastore:operational supports query operations. - * - * @param cmHandleId the cm handle - * @param resourceIdentifier the resource identifier - * @param includeDescendants whether include descendants - * @return the response entity - */ - public ResponseEntity executeRequest(final String cmHandleId, - final String resourceIdentifier, - final boolean includeDescendants) { - - final Supplier taskSupplier = getTaskSupplierForQueryRequest(cmHandleId, resourceIdentifier, - includeDescendants); - return executeTaskSync(taskSupplier); - } - /** - * Executes asynchronous request for group of cm handles to resource data. - * - * @param topicParamInQuery the topic param in query - * @param dataOperationRequest data operation request details for resource data - * @return the response entity - */ - public ResponseEntity executeRequest(final String topicParamInQuery, - final DataOperationRequest - dataOperationRequest) { - validateDataOperationRequest(topicParamInQuery, dataOperationRequest); - if (!notificationFeatureEnabled) { - return ResponseEntity.ok(Map.of("status", - "Asynchronous request is unavailable as notification feature is currently disabled.")); - } - return getRequestIdAndSendDataOperationRequestToDmiService(topicParamInQuery, dataOperationRequest); - } - - protected ResponseEntity executeTaskAsync(final String topicParamInQuery, + private ResponseEntity executeTaskAsync(final String topicParamInQuery, final String requestId, final Supplier taskSupplier) { - TopicValidator.validateTopicName(topicParamInQuery); log.debug("Received Async request with id {}", requestId); cpsNcmpTaskExecutor.executeTask(taskSupplier, timeOutInMilliSeconds); @@ -145,33 +103,15 @@ public class NcmpDatastoreRequestHandler implements TaskManagementDefaultHandler final String requestId = UUID.randomUUID().toString(); final Supplier taskSupplier = getTaskSupplierForGetRequest(datastoreName, cmHandleId, resourceIdentifier, optionsParamInQuery, topicParamInQuery, requestId, includeDescendants); - if (taskSupplier == NO_OBJECT_SUPPLIER) { - return new ResponseEntity<>(Map.of("status", "Unable to execute request as " - + "datastore is not implemented."), HttpStatus.NOT_IMPLEMENTED); - } return executeTaskAsync(topicParamInQuery, requestId, taskSupplier); } - private ResponseEntity getRequestIdAndSendDataOperationRequestToDmiService(final String topicParamInQuery, - final DataOperationRequest - dataOperationRequest) { - final String requestId = UUID.randomUUID().toString(); - sendDataOperationRequestAsynchronously(topicParamInQuery, dataOperationRequest, requestId); - return ResponseEntity.ok(Map.of("requestId", requestId)); - } + protected abstract Supplier getTaskSupplierForGetRequest(final String datastoreName, + final String cmHandleId, + final String resourceIdentifier, + final String optionsParamInQuery, + final String topicParamInQuery, + final String requestId, + final boolean includeDescendant); - private void validateDataOperationRequest(final String topicParamInQuery, - 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) { - throw new InvalidDatastoreException(dataOperationDetail.getDatastore() - + " datastore is not supported"); - } - }); - } } 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 0e49c6df1..8a3257576 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 @@ -20,11 +20,21 @@ package org.onap.cps.ncmp.rest.controller.handlers; +import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.OPERATIONAL; +import static org.onap.cps.ncmp.api.impl.operations.OperationType.READ; + +import java.util.Map; +import java.util.UUID; import java.util.function.Supplier; import org.onap.cps.ncmp.api.NetworkCmProxyDataService; +import org.onap.cps.ncmp.api.impl.exception.InvalidDatastoreException; +import org.onap.cps.ncmp.api.impl.operations.DatastoreType; +import org.onap.cps.ncmp.api.impl.operations.OperationType; import org.onap.cps.ncmp.api.models.DataOperationRequest; +import org.onap.cps.ncmp.rest.exceptions.OperationNotSupportedException; import org.onap.cps.ncmp.rest.executor.CpsNcmpTaskExecutor; -import org.springframework.scheduling.annotation.Async; +import org.onap.cps.ncmp.rest.util.TopicValidator; +import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Component; @Component @@ -32,6 +42,8 @@ public class NcmpPassthroughResourceRequestHandler extends NcmpDatastoreRequestH private final NetworkCmProxyDataService networkCmProxyDataService; + private static final Object noReturn = null; + /** * Constructor. * @@ -44,27 +56,71 @@ public class NcmpPassthroughResourceRequestHandler extends NcmpDatastoreRequestH this.networkCmProxyDataService = networkCmProxyDataService; } + /** + * Executes asynchronous request for group of cm handles to resource data. + * + * @param topicParamInQuery the topic param in query + * @param dataOperationRequest data operation request details for resource data + * @return the response entity + */ + public ResponseEntity executeRequest(final String topicParamInQuery, + final DataOperationRequest + dataOperationRequest) { + validateDataOperationRequest(topicParamInQuery, dataOperationRequest); + if (!notificationFeatureEnabled) { + return ResponseEntity.ok(Map.of("status", + "Asynchronous request is unavailable as notification feature is currently disabled.")); + } + return getRequestIdAndSendDataOperationRequestToDmiService(topicParamInQuery, dataOperationRequest); + } + @Override - public Supplier getTaskSupplierForGetRequest(final String datastoreName, - final String cmHandleId, - final String resourceIdentifier, - final String optionsParamInQuery, - final String topicParamInQuery, - final String requestId, - final boolean includeDescendants) { + protected Supplier getTaskSupplierForGetRequest(final String datastoreName, + final String cmHandleId, + final String resourceIdentifier, + final String optionsParamInQuery, + final String topicParamInQuery, + final String requestId, + final boolean includeDescendants) { return () -> networkCmProxyDataService.getResourceDataForCmHandle( - datastoreName, cmHandleId, resourceIdentifier, optionsParamInQuery, topicParamInQuery, requestId); + datastoreName, cmHandleId, resourceIdentifier, optionsParamInQuery, topicParamInQuery, requestId); } - @Async - @Override - public void sendDataOperationRequestAsynchronously(final String topicParamInQuery, - final DataOperationRequest - dataOperationRequest, - final String requestId) { - networkCmProxyDataService.executeDataOperationForCmHandles(topicParamInQuery, dataOperationRequest, - requestId); + private ResponseEntity getRequestIdAndSendDataOperationRequestToDmiService(final String topicParamInQuery, + final DataOperationRequest + dataOperationRequest) { + final String requestId = UUID.randomUUID().toString(); + cpsNcmpTaskExecutor.executeTask( + getTaskSupplierForDataOperationRequest(topicParamInQuery, dataOperationRequest, requestId), + timeOutInMilliSeconds); + return ResponseEntity.ok(Map.of("requestId", requestId)); + } + private void validateDataOperationRequest(final String topicParamInQuery, + 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) { + throw new InvalidDatastoreException(dataOperationDetail.getDatastore() + + " datastore is not supported"); + } + }); } + + private Supplier getTaskSupplierForDataOperationRequest(final String topicParamInQuery, + final DataOperationRequest dataOperationRequest, + final String requestId) { + return () -> { + networkCmProxyDataService.executeDataOperationForCmHandles(topicParamInQuery, + dataOperationRequest, + requestId); + return noReturn; + }; + } + } diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/TaskManagementDefaultHandler.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/TaskManagementDefaultHandler.java deleted file mode 100644 index b2520b160..000000000 --- a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/handlers/TaskManagementDefaultHandler.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * ============LICENSE_START======================================================= - * Copyright (C) 2023 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.controller.handlers; - -import java.util.function.Supplier; -import org.onap.cps.ncmp.api.models.DataOperationRequest; -import org.onap.cps.spi.FetchDescendantsOption; - -public interface TaskManagementDefaultHandler { - - String NO_REQUEST_ID = null; - String NO_TOPIC = null; - Supplier NO_OBJECT_SUPPLIER = null; - - default Supplier getTaskSupplierForGetRequest(final String datastoreName, - final String cmHandleId, - final String resourceIdentifier, - final String optionsParamInQuery, - final String topicParamInQuery, - final String requestId, - final boolean includeDescendant) { - return NO_OBJECT_SUPPLIER; - } - - default Supplier getTaskSupplierForQueryRequest(final String cmHandleId, - final String resourceIdentifier, - final boolean includeDescendant) { - return NO_OBJECT_SUPPLIER; - } - - default void sendDataOperationRequestAsynchronously(final String topicParamInQuery, - final DataOperationRequest - dataOperationRequest, - final String requestId) { - } - - static FetchDescendantsOption getFetchDescendantsOption(final boolean includeDescendants) { - return includeDescendants ? FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS - : FetchDescendantsOption.OMIT_DESCENDANTS; - } -} diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/executor/CpsNcmpTaskExecutor.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/executor/CpsNcmpTaskExecutor.java index 0543c4fba..ba68d5b75 100644 --- a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/executor/CpsNcmpTaskExecutor.java +++ b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/executor/CpsNcmpTaskExecutor.java @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2022 Nordix Foundation + * Copyright (C) 2022-2023 Nordix Foundation * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,19 +35,19 @@ public class CpsNcmpTaskExecutor { * Execute a task asynchronously. * * @param taskSupplier functional method is get() task need to executed asynchronously - * @param timeOutInMillis the time out value in milliseconds + * @param timeOutInMillis the time-out value in milliseconds */ - public void executeTask(final Supplier taskSupplier, final int timeOutInMillis) { + public void executeTask(final Supplier taskSupplier, final long timeOutInMillis) { CompletableFuture.supplyAsync(taskSupplier::get) .orTimeout(timeOutInMillis, MILLISECONDS) - .whenCompleteAsync((responseAsJson, throwable) -> handleTaskCompletion(throwable)); + .whenCompleteAsync((taskResult, throwable) -> handleTaskCompletion(throwable)); } private void handleTaskCompletion(final Throwable throwable) { if (throwable == null) { log.info("Async task completed successfully."); } else { - log.error("Async task failed. caused by : {}", throwable.getMessage()); + log.error("Async task failed. caused by : {}", throwable.toString()); } } } -- cgit 1.2.3-korg