diff options
author | ToineSiebelink <toine.siebelink@est.tech> | 2023-07-18 13:23:29 +0100 |
---|---|---|
committer | ToineSiebelink <toine.siebelink@est.tech> | 2023-07-20 13:19:28 +0100 |
commit | 5fd49caaec2ec7ab5424dc0f26209d437c7387cd (patch) | |
tree | 1713d335862cee083535125fea1802f05f4f3b90 /cps-ncmp-rest/src/test/groovy/org | |
parent | dcf84ad73f0301ef41049e692b9963f6dcac3661 (diff) |
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 <toine.siebelink@est.tech>
Change-Id: Ibe601c709de65080fa8898f2419fdbd92c5ba27d
Diffstat (limited to 'cps-ncmp-rest/src/test/groovy/org')
4 files changed, 240 insertions, 68 deletions
diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyControllerSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyControllerSpec.groovy index 4ee31e1ec5..7964e32b6f 100644 --- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyControllerSpec.groovy +++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyControllerSpec.groovy @@ -75,7 +75,6 @@ import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.OPERATIONAL import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS; import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS; - @WebMvcTest(NetworkCmProxyController) class NetworkCmProxyControllerSpec extends Specification { @@ -104,16 +103,16 @@ class NetworkCmProxyControllerSpec extends Specification { DataOperationRequestMapper dataOperationRequestMapper = Mappers.getMapper(DataOperationRequestMapper) @SpringBean - CpsNcmpTaskExecutor spiedCpsTaskExecutor = Spy() + CpsNcmpTaskExecutor mockCpsTaskExecutor = Mock() @SpringBean DeprecationHelper stubbedDeprecationHelper = Stub() @SpringBean - NcmpCachedResourceRequestHandler ncmpCachedResourceRequestHandler = new NcmpCachedResourceRequestHandler(spiedCpsTaskExecutor, mockNetworkCmProxyDataService, mockNetworkCmProxyQueryService) + NcmpCachedResourceRequestHandler ncmpCachedResourceRequestHandler = new NcmpCachedResourceRequestHandler(mockCpsTaskExecutor, mockNetworkCmProxyDataService, mockNetworkCmProxyQueryService) @SpringBean - NcmpPassthroughResourceRequestHandler ncmpPassthroughResourceRequestHandler = new NcmpPassthroughResourceRequestHandler(spiedCpsTaskExecutor, mockNetworkCmProxyDataService) + NcmpPassthroughResourceRequestHandler ncmpPassthroughResourceRequestHandler = new NcmpPassthroughResourceRequestHandler(mockCpsTaskExecutor, mockNetworkCmProxyDataService) @Value('${rest.api.ncmp-base-path}/v1') def ncmpBasePathV1 @@ -150,23 +149,6 @@ class NetworkCmProxyControllerSpec extends Specification { response.status == HttpStatus.OK.value() } - def 'Get Resource Data Async Topic Handling with #scenario.'() { - given: 'resource data url' - def getUrl = "$ncmpBasePathV1/ch/testCmHandle/data/ds/ncmp-datastore:passthrough-operational?resourceIdentifier=parent/child&${topicQueryParam}" - when: 'get data resource request is performed' - def response = mvc.perform( - get(getUrl).contentType(MediaType.APPLICATION_JSON)).andReturn().response - then: 'task executor is called appropriate number of times' - expectedNumberOfTaskExecutions * spiedCpsTaskExecutor.executeTask(_, TIMOUT_FOR_TEST) - and: 'response status is OK' - response.status == HttpStatus.OK.value() - where: 'the following parameters are used' - scenario | datastoreInUrl | topicQueryParam || expectedNumberOfTaskExecutions - 'url with valid topic' | 'passthrough-operational' | '&topic=my-topic-name' || 1 - 'no topic in url' | 'passthrough-operational' | '' || 0 - 'null topic in url' | 'passthrough-operational' | '&topic=null' || 1 - } - def 'Get Resource Data from ncmp-datastore:operational (cached) parameters handling with #scenario.'() { given: 'resource data url' def getUrl = "$ncmpBasePathV1/ch/h123/data/ds/ncmp-datastore:operational" + @@ -188,30 +170,10 @@ class NetworkCmProxyControllerSpec extends Specification { 'options (ignored)' | '&options=(a-=1)' || OMIT_DESCENDANTS } - def 'Get Resource Data with invalid topic parameter: #scenario.'() { - given: 'resource data url' - def getUrl = "$ncmpBasePathV1/ch/testCmHandle/data/ds/ncmp-datastore:${datastoreInUrl}" + - "?resourceIdentifier=parent/child&options=(a=1,b=2)${topicQueryParam}" - when: 'get data resource (async) request is performed' - def response = mvc.perform( - get(getUrl).contentType(MediaType.APPLICATION_JSON)).andReturn().response - then: 'abad request is returned' - response.status == HttpStatus.BAD_REQUEST.value() - where: 'the following parameters are used' - scenario | datastoreInUrl | topicQueryParam - 'empty topic in url' | 'passthrough-operational' | '&topic=\"\"' - 'missing topic in url' | 'passthrough-operational' | '&topic=' - 'blank topic value in url' | 'passthrough-operational' | '&topic=\" \"' - 'invalid non-empty topic value in url' | 'passthrough-operational' | '&topic=1_5_*_#' - } - def 'Execute (async) data operation to read data from dmi service.'() { given: 'data operation url' def getUrl = "$ncmpBasePathV1/data?topic=my-topic-name" - def dataOperationRequestJsonData = jsonObjectMapper.asJsonString( - getDataOperationRequest("read", datastore.datastoreName)) - def expectedDmiDataOperationRequest - = jsonObjectMapper.convertJsonString(dataOperationRequestJsonData, org.onap.cps.ncmp.api.models.DataOperationRequest.class) + def dataOperationRequestJsonData = jsonObjectMapper.asJsonString(getDataOperationRequest("read", datastore.datastoreName)) when: 'post data operation request is performed' def response = mvc.perform( post(getUrl) @@ -221,20 +183,18 @@ class NetworkCmProxyControllerSpec extends Specification { then: 'response status is Ok' response.status == HttpStatus.OK.value() and: 'async request id is generated' - assert response.contentAsString.contains("requestId") - then: 'wait a little to allow execution of service method by task executor (on separate thread)' - Thread.sleep(100) - then: 'the service has been invoked with the correct parameters ' - 1 * mockNetworkCmProxyDataService.executeDataOperationForCmHandles('my-topic-name', expectedDmiDataOperationRequest, _) + assert response.contentAsString.contains('requestId') + then: 'the request is handled asynchronously' + 1 * mockCpsTaskExecutor.executeTask(*_) where: 'the following data stores are used' datastore << [PASSTHROUGH_RUNNING, PASSTHROUGH_OPERATIONAL] } - def 'Execute (async) data operation for #scenario from dmi service.'() { + def 'Execute (async) data operation with some validation error.'() { given: 'data operation url' def getUrl = "$ncmpBasePathV1/data?topic=my-topic-name" def dataOperationRequestJsonData = jsonObjectMapper.asJsonString( - getDataOperationRequest(operation, datastore)) + getDataOperationRequest('read', 'invalid datastore')) when: 'post data resource request is performed' def response = mvc.perform( post(getUrl) @@ -243,18 +203,13 @@ class NetworkCmProxyControllerSpec extends Specification { ).andReturn().response then: 'response status is BAD_REQUEST' response.status == HttpStatus.BAD_REQUEST.value() - where: 'the following parameters are used' - scenario | datastore | operation - 'non-supported datastoreName' | OPERATIONAL.datastoreName | 'read' - 'non-supported operation (passthrough-running)' | PASSTHROUGH_RUNNING.datastoreName | 'create' - 'non-supported operation (passthrough-operational)' | PASSTHROUGH_OPERATIONAL.datastoreName | 'create' } def 'Get data operation resource data when notification feature is disabled for datastore: #datastore.'() { given: 'data operation url' def getUrl = "$ncmpBasePathV1/data?topic=my-topic-name" def dataOperationRequestJsonData = jsonObjectMapper.asJsonString( - getDataOperationRequest("read", datastore.datastoreName)) + getDataOperationRequest("read", PASSTHROUGH_RUNNING.datastoreName)) ncmpPassthroughResourceRequestHandler.notificationFeatureEnabled = false when: 'post data resource request is performed' def response = mvc.perform( @@ -266,8 +221,6 @@ class NetworkCmProxyControllerSpec extends Specification { response.status == HttpStatus.OK.value() and: 'async request id is unavailable' assert response.contentAsString == '{"status":"Asynchronous request is unavailable as notification feature is currently disabled."}' - where: 'the following data stores are used' - datastore << [PASSTHROUGH_RUNNING, PASSTHROUGH_OPERATIONAL] } def 'Query Resource Data from operational.'() { @@ -287,9 +240,9 @@ class NetworkCmProxyControllerSpec extends Specification { response.status == HttpStatus.OK.value() } - def 'Query Resource Data using datastore of #datastore'() { + def 'Query Resource Data with unsupported datastore'() { given: 'the query resource data url' - def getUrl = "$ncmpBasePathV1/ch/testCmHandle/data/ds/ncmp-datastore:${datastore}/query" + + def getUrl = "$ncmpBasePathV1/ch/testCmHandle/data/ds/ncmp-datastore:passthrough-running/query" + "?cps-path=/cps/path" when: 'the query data resource request is performed' def response = mvc.perform( @@ -298,10 +251,8 @@ class NetworkCmProxyControllerSpec extends Specification { ).andReturn().response then: 'a 400 BAD_REQUEST is returned for the unsupported datastore' response.status == 400 - and: 'the error message is that datastore #datastore is not supported' - response.contentAsString.contains("ncmp-datastore:${datastore} is not supported") - where: 'the following datastore is used' - datastore << ["passthrough-running", "passthrough-operational"] + and: 'the error message is that the datastore is not supported' + response.contentAsString.contains("ncmp-datastore:passthrough-running is not supported") } def 'Get Resource Data from pass-through running with #scenario value in resource identifier param.'() { @@ -586,7 +537,7 @@ class NetworkCmProxyControllerSpec extends Specification { def 'Get Resource Data from operational with or without descendants'() { given: 'resource data url with descendants #enabled' def getUrl = "$ncmpBasePathV1/ch/testCmHandle/data/ds/ncmp-datastore:operational" + - "?resourceIdentifier=parent/child&include-descendants=${enabled}" + "?resourceIdentifier=parent/child&include-descendants=${booleanValue}" when: 'get data resource request is performed' def response = mvc.perform( get(getUrl) @@ -597,9 +548,9 @@ class NetworkCmProxyControllerSpec extends Specification { and: 'response status is Ok' response.status == HttpStatus.OK.value() where: 'the following parameters are used' - enabled | descendantsOption - false | FetchDescendantsOption.OMIT_DESCENDANTS - true | FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS + booleanValue | descendantsOption + false | OMIT_DESCENDANTS + true | INCLUDE_ALL_DESCENDANTS } def 'Attempt execute #operation rest operation on resource data with #scenario'() { 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 300026d34a..e755094dd2 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 @@ -251,7 +251,7 @@ class NetworkCmProxyInventoryControllerSpec extends Specification { } def failedResponse(cmHandle) { - return CmHandleRegistrationResponse.createFailureResponse(cmHandle, new RuntimeException("Failed")) + return CmHandleRegistrationResponse.createFailureResponse(cmHandle, new RuntimeException('Failed')) } def successResponse(cmHandle) { 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 new file mode 100644 index 0000000000..e149cb7c4a --- /dev/null +++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandlerSpec.groovy @@ -0,0 +1,135 @@ +/* + * ============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 org.onap.cps.ncmp.api.NetworkCmProxyDataService +import org.onap.cps.ncmp.api.impl.exception.InvalidDatastoreException +import org.onap.cps.ncmp.api.impl.exception.InvalidOperationException +import org.onap.cps.ncmp.api.models.DataOperationDefinition +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 spock.lang.Specification +import spock.util.concurrent.PollingConditions + +class NcmpDatastoreRequestHandlerSpec extends Specification { + + def spiedCpsNcmpTaskExecutor = Spy(CpsNcmpTaskExecutor) + def mockNetworkCmProxyDataService = Mock(NetworkCmProxyDataService) + + def objectUnderTest = new NcmpPassthroughResourceRequestHandler(spiedCpsNcmpTaskExecutor, mockNetworkCmProxyDataService) + + def 'Attempt to execute async get request with #scenario.'() { + given: 'notification feature is turned on/off' + objectUnderTest.notificationFeatureEnabled = notificationFeatureEnabled + when: 'get request is executed with topic = #topic' + objectUnderTest.executeRequest('ds', 'ch1', 'resource1', 'options', topic, false) + and: 'wait a little for async execution (only if expected)' + if (expectedCalls > 0) { + Thread.sleep(100) + } + then: 'the task is executed in an async fashion or not' + expectedCalls * spiedCpsNcmpTaskExecutor.executeTask(*_) + and: 'the service request is always invoked' + 1 * mockNetworkCmProxyDataService.getResourceDataForCmHandle('ds', 'ch1', 'resource1', 'options', _, _) + where: 'the following parameters are used' + scenario | notificationFeatureEnabled | topic || expectedCalls + 'feature on, valid topic' | true | 'valid' || 1 + 'feature on, no topic' | true | null || 0 + 'feature off, valid topic' | false | 'valid' || 0 + 'feature off, no topic' | false | null || 0 + } + + def 'Attempt to execute async data operation request with feature #scenario.'() { + given: 'a extended request handler that supports bulk requests' + def objectUnderTest = new NcmpPassthroughResourceRequestHandler(spiedCpsNcmpTaskExecutor, mockNetworkCmProxyDataService) + and: 'notification feature is turned on/off' + objectUnderTest.notificationFeatureEnabled = notificationFeatureEnabled + when: 'data operation request is executed' + objectUnderTest.executeRequest('someTopic', new DataOperationRequest()) + then: 'the task is executed in an async fashion or not' + expectedCalls * spiedCpsNcmpTaskExecutor.executeTask(*_) + where: 'the following parameters are used' + scenario | notificationFeatureEnabled || expectedCalls + 'on' | true || 1 + 'off' | false || 0 + } + + def 'Execute async data operation request with datastore #datastore.'() { + given: 'notification feature is turned on' + objectUnderTest.notificationFeatureEnabled = true + and: 'a data operation request with datastore: #datastore' + def dataOperationDefinition = new DataOperationDefinition(operation: 'read', datastore: datastore) + def dataOperationRequest = new DataOperationRequest(dataOperationDefinitions: [dataOperationDefinition]) + and: ' a flag to track the network service call' + def networkServiceMethodCalled = false + and: 'the (mocked) service will use the flag to indicate it is called' + mockNetworkCmProxyDataService.executeDataOperationForCmHandles('myTopic', dataOperationRequest, _) >> { + networkServiceMethodCalled = true + } + when: 'data operation request is executed' + objectUnderTest.executeRequest('myTopic', dataOperationRequest) + then: 'the task is executed in an async fashion' + 1 * spiedCpsNcmpTaskExecutor.executeTask(*_) + and: 'the network service is invoked (wait max. 5 seconds)' + new PollingConditions(timeout: 5).eventually { + networkServiceMethodCalled == true + } + where: 'the following datastores are used' + datastore << ['ncmp-datastore:passthrough-running', 'ncmp-datastore:passthrough-operational'] + } + + 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' + def dataOperationDefinition = new DataOperationDefinition(operation: 'read', datastore: datastore) + when: 'data operation request is executed' + def dataOperationRequest = new DataOperationRequest(dataOperationDefinitions: [dataOperationDefinition]) + objectUnderTest.executeRequest('myTopic', dataOperationRequest) + then: 'the correct error is thrown' + def thrown = thrown(InvalidDatastoreException) + assert thrown.message.contains(expectedErrorMessage) + where: 'the following datastore names are used' + scenario | datastore || expectedErrorMessage + 'unsupported datastore' | 'ncmp-datastore:operational' || 'not supported' + 'invalid datastore' | 'invalid' || 'invalid datastore name' + } + + 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' + def dataOperationDefinition = new DataOperationDefinition(operation: operation, datastore: 'ncmp-datastore:passthrough-running') + when: 'bulk request is executed' + objectUnderTest.executeRequest('someTopic', new DataOperationRequest(dataOperationDefinitions:[dataOperationDefinition])) + then: 'the expected type of exception is thrown' + thrown(expectedException) + where: 'the following operations are used' + scenario | operation || expectedException + 'invalid' | 'invalid' || InvalidOperationException + 'unsupported' | 'create' || OperationNotSupportedException + 'unsupported' | 'update' || OperationNotSupportedException + 'unsupported' | 'patch' || OperationNotSupportedException + 'unsupported' | 'delete' || OperationNotSupportedException + } + +} diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/executor/CpsNcmpTaskExecutorSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/executor/CpsNcmpTaskExecutorSpec.groovy new file mode 100644 index 0000000000..870c36c777 --- /dev/null +++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/executor/CpsNcmpTaskExecutorSpec.groovy @@ -0,0 +1,86 @@ +/* + * ============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.executor + +import ch.qos.logback.classic.Level +import ch.qos.logback.classic.Logger +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.read.ListAppender +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.onap.cps.notification.NotificationErrorHandler +import org.slf4j.LoggerFactory +import spock.lang.Specification + +class CpsNcmpTaskExecutorSpec extends Specification { + + def objectUnderTest = new CpsNcmpTaskExecutor() + def logger = Spy(ListAppender<ILoggingEvent>) + def enoughTime = 100 + + @BeforeEach + void setup() { + ((Logger) LoggerFactory.getLogger(CpsNcmpTaskExecutor.class)).addAppender(logger); + logger.start(); + } + + @AfterEach + void teardown() { + ((Logger) LoggerFactory.getLogger(NotificationErrorHandler.class)).detachAndStopAllAppenders(); + } + + def 'Execute successful task.'() { + when: 'task is executed' + objectUnderTest.executeTask(taskSupplier(), enoughTime) + and: 'wait a little for async execution completion' + Thread.sleep(10) + then: 'an event is logged with level INFO' + def loggingEvent = getLoggingEvent() + assert loggingEvent.level == Level.INFO + and: 'the log indicates the task completed successfully' + assert loggingEvent.formattedMessage == 'Async task completed successfully.' + } + + def 'Execute failing task.'() { + when: 'task is executed' + objectUnderTest.executeTask(taskSupplierForFailingTask(), enoughTime) + and: 'wait a little for async execution completion' + Thread.sleep(10) + then: 'an event is logged with level ERROR' + def loggingEvent = getLoggingEvent() + assert loggingEvent.level == Level.ERROR + and: 'the original error message is logged' + assert loggingEvent.formattedMessage.contains('original exception message') + } + + def taskSupplier() { + return () -> 'hello world' + } + + def taskSupplierForFailingTask() { + return () -> { throw new RuntimeException('original exception message') } + } + + def getLoggingEvent() { + return logger.list[0] + } + +} |