Refactor and Move NCMP Data Request Handlers

- Simplified NCMP Rest Request Handlers
- Moved responsEntity wrapping to Controller so most handler methods can have clear return types
- Moved NCMP Rest Request Handlers  to Service Layer
- Moved related exceptions and utils to Service Layer
- Used Lombok for constructors
- Improved related testware

Issue-ID: CPS-2266
Change-Id: I0025fab1c92e0d613825093b6e4b43dae044c01a
Signed-off-by: ToineSiebelink <toine.siebelink@est.tech>

4 files changed, 41 insertions, 237 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 3a5748f002..34b9dbe950 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
@@ -23,42 +23,26 @@
 
 package org.onap.cps.ncmp.rest.controller
 
-import static org.onap.cps.ncmp.api.impl.inventory.CompositeState.DataStores
-import static org.onap.cps.ncmp.api.impl.inventory.CompositeState.Operational
-import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.OPERATIONAL
-import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.PASSTHROUGH_OPERATIONAL
-import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.PASSTHROUGH_RUNNING
-import static org.onap.cps.ncmp.api.impl.operations.OperationType.CREATE
-import static org.onap.cps.ncmp.api.impl.operations.OperationType.DELETE
-import static org.onap.cps.ncmp.api.impl.operations.OperationType.PATCH
-import static org.onap.cps.ncmp.api.impl.operations.OperationType.UPDATE
-import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS
-import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put
-
 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 com.fasterxml.jackson.databind.ObjectMapper
+import groovy.json.JsonSlurper
 import org.mapstruct.factory.Mappers
 import org.onap.cps.TestUtils
 import org.onap.cps.events.EventsPublisher
 import org.onap.cps.ncmp.api.NetworkCmProxyDataService
 import org.onap.cps.ncmp.api.NetworkCmProxyQueryService
+import org.onap.cps.ncmp.api.impl.NcmpCachedResourceRequestHandler
+import org.onap.cps.ncmp.api.impl.NcmpPassthroughResourceRequestHandler
 import org.onap.cps.ncmp.api.impl.inventory.CmHandleState
 import org.onap.cps.ncmp.api.impl.inventory.CompositeState
 import org.onap.cps.ncmp.api.impl.inventory.DataStoreSyncState
 import org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory
 import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevel
-import org.onap.cps.ncmp.api.models.NcmpServiceCmHandle
 import org.onap.cps.ncmp.api.models.CmResourceAddress
-import org.onap.cps.ncmp.rest.controller.handlers.NcmpCachedResourceRequestHandler
-import org.onap.cps.ncmp.rest.controller.handlers.NcmpPassthroughResourceRequestHandler
+import org.onap.cps.ncmp.api.models.NcmpServiceCmHandle
 import org.onap.cps.ncmp.rest.mapper.CmHandleStateMapper
 import org.onap.cps.ncmp.rest.mapper.DataOperationRequestMapper
 import org.onap.cps.ncmp.rest.model.DataOperationDefinition
@@ -75,15 +59,32 @@ import org.springframework.beans.factory.annotation.Value
 import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest
 import org.springframework.http.HttpStatus
 import org.springframework.http.MediaType
+import org.springframework.http.ResponseEntity
 import org.springframework.test.web.servlet.MockMvc
+import reactor.core.publisher.Mono
 import spock.lang.Shared
 import spock.lang.Specification
+
 import java.time.OffsetDateTime
 import java.time.ZoneOffset
 import java.time.format.DateTimeFormatter
-import groovy.json.JsonSlurper
-import org.springframework.http.ResponseEntity
-import reactor.core.publisher.Mono
+
+import static org.onap.cps.ncmp.api.impl.inventory.CompositeState.DataStores
+import static org.onap.cps.ncmp.api.impl.inventory.CompositeState.Operational
+import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.OPERATIONAL
+import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.PASSTHROUGH_OPERATIONAL
+import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.PASSTHROUGH_RUNNING
+import static org.onap.cps.ncmp.api.impl.operations.OperationType.CREATE
+import static org.onap.cps.ncmp.api.impl.operations.OperationType.DELETE
+import static org.onap.cps.ncmp.api.impl.operations.OperationType.PATCH
+import static org.onap.cps.ncmp.api.impl.operations.OperationType.UPDATE
+import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS
+import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put
 
 @WebMvcTest(NetworkCmProxyController)
 class NetworkCmProxyControllerSpec extends Specification {
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
deleted file mode 100644
index 00b0cb04c5..0000000000
--- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/handlers/NcmpDatastoreRequestHandlerSpec.groovy
+++ /dev/null
@@ -1,152 +0,0 @@
-/*
- *  ============LICENSE_START=======================================================
- *  Copyright (C) 2023-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.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.api.models.CmResourceAddress
-import org.onap.cps.ncmp.rest.exceptions.OperationNotSupportedException
-import org.onap.cps.ncmp.rest.exceptions.PayloadTooLargeException
-import org.springframework.http.HttpStatus
-import reactor.core.publisher.Mono
-import spock.lang.Specification
-import spock.util.concurrent.PollingConditions
-
-class NcmpDatastoreRequestHandlerSpec extends Specification {
-
-    def mockNetworkCmProxyDataService = Mock(NetworkCmProxyDataService)
-
-    def objectUnderTest = new NcmpPassthroughResourceRequestHandler(mockNetworkCmProxyDataService)
-
-    def NO_AUTH_HEADER = null
-
-    def setup() {
-        objectUnderTest.timeOutInMilliSeconds = 100
-    }
-
-    def 'Attempt to execute async get request with #scenario.'() {
-        given: 'notification feature is turned on/off'
-            objectUnderTest.notificationFeatureEnabled = notificationFeatureEnabled
-        and: 'a CM resource address'
-            def cmResourceAddress = new CmResourceAddress('ds', 'ch1', 'resource1')
-        and: 'the (mocked) service is called with the correct parameters returns OK'
-            1 * mockNetworkCmProxyDataService.getResourceDataForCmHandle(cmResourceAddress, 'options', _, _, NO_AUTH_HEADER) >> Mono.just(HttpStatus.OK)
-        when: 'get request is executed with topic = #topic'
-            def response= objectUnderTest.executeRequest(cmResourceAddress, 'options', topic, false, NO_AUTH_HEADER)
-        then: 'a successful response with/without request id is returned'
-            assert response.statusCode.value == 200
-            assert response.body instanceof Map == expectedResponseBodyIsMap
-        where: 'the following parameters are used'
-            scenario                   | notificationFeatureEnabled | topic   || expectedCalls | expectedResponseBodyIsMap
-            'feature on, valid topic'  | true                       | 'valid' || 1             | true
-            'feature on, no topic'     | true                       | null    || 0             | false
-            'feature off, valid topic' | false                      | 'valid' || 0             | false
-            'feature off, no topic'    | false                      | null    || 0             | false
-    }
-
-    def 'Attempt to execute async data operation request with feature #scenario.'() {
-        given: 'a extended request handler that supports bulk requests'
-           def objectUnderTest = new NcmpPassthroughResourceRequestHandler(mockNetworkCmProxyDataService)
-        and: 'notification feature is turned on/off'
-            objectUnderTest.notificationFeatureEnabled = notificationFeatureEnabled
-        when: 'data operation request is executed'
-            objectUnderTest.executeRequest('someTopic', new DataOperationRequest(), NO_AUTH_HEADER)
-        then: 'the task is executed in an async fashion or not'
-            expectedCalls * mockNetworkCmProxyDataService.executeDataOperationForCmHandles('someTopic', _, _, null)
-        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, _, NO_AUTH_HEADER) >> {
-                networkServiceMethodCalled = true
-            }
-        when: 'data operation request is executed'
-            def response = objectUnderTest.executeRequest('myTopic', dataOperationRequest, NO_AUTH_HEADER)
-        and: 'a successful response with request id is returned'
-            assert response.statusCode.value == 200
-            assert response.body.requestId != null
-        then: 'the network service is invoked'
-            new PollingConditions().within(1) {
-                assert 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: '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, NO_AUTH_HEADER)
-        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: 'a data operation definition with operation: #operation'
-            def dataOperationDefinition = new DataOperationDefinition(operation: operation, datastore: 'ncmp-datastore:passthrough-running')
-        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)
-        where: 'the following operations are used'
-            scenario      | operation || expectedException
-            'invalid'     | 'invalid' || InvalidOperationException
-            'unsupported' | 'create'  || OperationNotSupportedException
-            'unsupported' | 'update'  || OperationNotSupportedException
-            'unsupported' | 'patch'   || OperationNotSupportedException
-            '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 tooMany = objectUnderTest.MAXIMUM_CM_HANDLES_PER_OPERATION+1
-            def cmHandleIds = new String[tooMany]
-            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 (${tooMany}) 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 33eb48ffa2..af8a8ea12a 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
@@ -21,28 +21,17 @@
 
 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.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
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post
-import static org.onap.cps.ncmp.api.NcmpResponseStatus.UNABLE_TO_READ_RESOURCE_DATA
-
 import groovy.json.JsonSlurper
 import org.mapstruct.factory.Mappers
 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.NcmpCachedResourceRequestHandler
+import org.onap.cps.ncmp.api.impl.NcmpPassthroughResourceRequestHandler
 import org.onap.cps.ncmp.api.impl.exception.DmiClientRequestException
+import org.onap.cps.ncmp.api.impl.exception.DmiRequestException
 import org.onap.cps.ncmp.api.impl.exception.ServerNcmpException
+import org.onap.cps.ncmp.exceptions.PayloadTooLargeException
 import org.onap.cps.ncmp.rest.controller.NcmpRestInputMapper
-import org.onap.cps.ncmp.rest.controller.handlers.NcmpCachedResourceRequestHandler
-import org.onap.cps.ncmp.rest.controller.handlers.NcmpPassthroughResourceRequestHandler
 import org.onap.cps.ncmp.rest.mapper.CmHandleStateMapper
 import org.onap.cps.ncmp.rest.mapper.DataOperationRequestMapper
 import org.onap.cps.ncmp.rest.util.DeprecationHelper
@@ -60,6 +49,18 @@ import org.springframework.test.web.servlet.MockMvc
 import spock.lang.Shared
 import spock.lang.Specification
 
+import static org.onap.cps.ncmp.api.NcmpResponseStatus.UNABLE_TO_READ_RESOURCE_DATA
+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_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.PAYLOAD_TOO_LARGE
+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 {
 
@@ -125,7 +126,7 @@ class NetworkCmProxyRestExceptionHandlerSpec extends Specification {
             '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'
+            'Operation too large' | new PayloadTooLargeException(sampleErrorMessage) || PAYLOAD_TOO_LARGE | sampleErrorMessage | 'Check logs'
     }
 
     def 'Post request with exception returns correct HTTP Status.'() {
diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/util/TopicValidatorSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/util/TopicValidatorSpec.groovy
deleted file mode 100644
index 15e2c1c6a0..0000000000
--- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/util/TopicValidatorSpec.groovy
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- *  ============LICENSE_START=======================================================
- *  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.
- *  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.util
-
-import org.onap.cps.ncmp.rest.exceptions.InvalidTopicException
-import spock.lang.Specification
-
-class TopicValidatorSpec extends Specification {
-
-    def 'Valid topic name validation.'() {
-        when: 'a valid topic name is validated'
-            TopicValidator.validateTopicName('my-valid-topic')
-        then: 'no exception is thrown'
-            noExceptionThrown()
-    }
-
-    def 'Validating invalid topic names.'() {
-        when: 'the invalid topic name is validated'
-            TopicValidator.validateTopicName(topicName)
-        then: 'boolean response will be returned for #scenario'
-            thrown(InvalidTopicException)
-        where: 'the following names are used'
-            scenario                  | topicName
-            'empty topic'             | ''
-            'blank topic'             | ' '
-            'invalid non empty topic' | '1_5_*_#'
-    }
-}