Reverted web client changes as it breaking CPS functionality:

1. Error handling not working
2. Unable to make dmi call as url appens extra "/"

Commit 1: Replace RestTemplate with WebClient in synchronous DMI calls

    - added DmiWebClientConfiguration
    - use WebClient in DmiRestClient
    - fixed unit tests
    - ingnored failing NCMP related integration tests (related to WebClient, will be fixed in a future patch)
    - encode query params for DMI requests

Commit 2: Module sync task failing with WebClient

    - added configurable buffer size

Commit 3: Re-use ncmp.dmi.httpclient.maximumConnectionsTotal parameter
(as documented in RTD) to control webclient connection poolsize

Issue-ID:CPS-989, CPS-2195, CPS-2198

Change-Id: I6c71ab2801b222708404f51bd60f2b3031ebf32e
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>

7 files changed, 38 insertions, 111 deletions

diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/client/DmiRestClientSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/client/DmiRestClientSpec.groovy
index 003dbf5b0..c8e34b1a5 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/client/DmiRestClientSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/client/DmiRestClientSpec.groovy
@@ -24,21 +24,20 @@ package org.onap.cps.ncmp.api.impl.client
 import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.fasterxml.jackson.databind.node.ObjectNode
-import org.onap.cps.ncmp.api.impl.config.DmiWebClientConfiguration;
+import org.onap.cps.ncmp.api.impl.config.NcmpConfiguration
+import org.onap.cps.ncmp.api.impl.config.NcmpConfiguration.DmiProperties;
 import org.onap.cps.ncmp.api.impl.exception.HttpClientRequestException
 import org.onap.cps.ncmp.utils.TestUtils
 import org.spockframework.spring.SpringBean
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.test.context.SpringBootTest
-
+import org.springframework.http.HttpEntity
 import org.springframework.http.HttpHeaders
 import org.springframework.http.HttpStatus
 import org.springframework.http.ResponseEntity
 import org.springframework.test.context.ContextConfiguration
-import org.springframework.test.context.TestPropertySource
 import org.springframework.web.client.HttpServerErrorException
-import org.springframework.web.reactive.function.client.WebClient
-import reactor.core.publisher.Mono
+import org.springframework.web.client.RestTemplate
 import spock.lang.Specification
 
 import static org.onap.cps.ncmp.api.impl.operations.OperationType.READ
@@ -46,49 +45,43 @@ import static org.onap.cps.ncmp.api.impl.operations.OperationType.PATCH
 import static org.onap.cps.ncmp.api.impl.operations.OperationType.CREATE
 
 @SpringBootTest
-@ContextConfiguration(classes = [DmiWebClientConfiguration, DmiRestClient, ObjectMapper])
+@ContextConfiguration(classes = [DmiProperties, DmiRestClient, ObjectMapper])
 class DmiRestClientSpec extends Specification {
 
     static final NO_AUTH_HEADER = null
     static final BASIC_AUTH_HEADER = 'Basic c29tZS11c2VyOnNvbWUtcGFzc3dvcmQ='
     static final BEARER_AUTH_HEADER = 'Bearer my-bearer-token'
 
+    @SpringBean
+    RestTemplate mockRestTemplate = Mock(RestTemplate)
+
     @Autowired
-    DmiWebClientConfiguration.DmiProperties dmiProperties
+    NcmpConfiguration.DmiProperties dmiProperties
 
     @Autowired
     DmiRestClient objectUnderTest
 
-    @SpringBean
-    WebClient mockWebClient = Mock(WebClient);
-
     @Autowired
     ObjectMapper objectMapper
 
-    def mockRequestBodyUriSpec = Mock(WebClient.RequestBodyUriSpec)
-    def mockResponseSpec = Mock(WebClient.ResponseSpec)
-    def mockResponseEntity = Mock(ResponseEntity)
-    def monoSpec = Mono.just(mockResponseEntity)
+    def responseFromRestTemplate = Mock(ResponseEntity)
+
     def 'DMI POST operation with JSON.'() {
-        given: 'the web client returns a valid response entity for the expected parameters'
-            mockWebClient.post() >> mockRequestBodyUriSpec
-            mockWebclientResponse()
-            mockRequestBodyUriSpec.body(_) >> mockRequestBodyUriSpec
-            mockResponseSpec.toEntity(Object.class) >> monoSpec
-            monoSpec.block() >> mockResponseEntity
+        given: 'the rest template returns a valid response entity for the expected parameters'
+            mockRestTemplate.postForEntity('my url', _ as HttpEntity, Object.class) >> responseFromRestTemplate
         when: 'POST operation is invoked'
-            def result = objectUnderTest.postOperationWithJsonData('/my/url', 'some json', READ, null)
+            def result = objectUnderTest.postOperationWithJsonData('my url', 'some json', READ, null)
         then: 'the output of the method is equal to the output from the test template'
-            result == mockResponseEntity
+            result == responseFromRestTemplate
     }
 
     def 'Failing DMI POST operation.'() {
         given: 'the rest template returns a valid response entity'
             def serverResponse = 'server response'.getBytes()
             def httpServerErrorException = new HttpServerErrorException(HttpStatus.FORBIDDEN, 'status text', serverResponse, null)
-            mockWebClient.post() >> { throw httpServerErrorException }
+            mockRestTemplate.postForEntity(*_) >> { throw httpServerErrorException }
         when: 'POST operation is invoked'
-            def result = objectUnderTest.postOperationWithJsonData('/some', 'some json', operation, null)
+            def result = objectUnderTest.postOperationWithJsonData('some url', 'some json', operation, null)
         then: 'a Http Client Exception is thrown'
             def thrown = thrown(HttpClientRequestException)
         and: 'the exception has the relevant details from the error response'
@@ -104,20 +97,16 @@ class DmiRestClientSpec extends Specification {
             def dmiPluginHealthCheckResponseJsonData = TestUtils.getResourceFileContent('dmiPluginHealthCheckResponse.json')
             def jsonNode = objectMapper.readValue(dmiPluginHealthCheckResponseJsonData, JsonNode.class)
             ((ObjectNode) jsonNode).put('status', 'my status')
-            def monoResponse = Mono.just(jsonNode)
-            mockWebClient.get() >> mockRequestBodyUriSpec
-            mockWebclientResponse()
-            mockResponseSpec.bodyToMono(_) >> monoResponse
-            monoResponse.block() >> jsonNode
+            mockRestTemplate.getForObject(*_) >> {jsonNode}
         when: 'get trust level of the dmi plugin'
-            def result = objectUnderTest.getDmiHealthStatus('some/url')
+            def result = objectUnderTest.getDmiHealthStatus('some url')
         then: 'the status value from the json is return'
             assert result == 'my status'
     }
 
     def 'Failing to get dmi plugin health status #scenario'() {
         given: 'rest template with #scenario'
-            mockWebClient.get() >> healthStatusResponse
+            mockRestTemplate.getForObject(*_) >> healthStatusResponse
         when: 'attempt to get health status of the dmi plugin'
             def result = objectUnderTest.getDmiHealthStatus('some url')
         then: 'result will be empty'
@@ -144,9 +133,4 @@ class DmiRestClientSpec extends Specification {
             'DMI basic auth disabled, with NCMP basic auth'   | false       | BASIC_AUTH_HEADER  || NO_AUTH_HEADER
     }
 
-    def mockWebclientResponse() {
-        mockRequestBodyUriSpec.uri(_) >> mockRequestBodyUriSpec
-        mockRequestBodyUriSpec.headers(_) >> mockRequestBodyUriSpec
-        mockRequestBodyUriSpec.retrieve() >> mockResponseSpec
-    }
 }
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/DmiWebClientConfigurationSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/DmiWebClientConfigurationSpec.groovy
deleted file mode 100644
index 93b3c6380..000000000
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/DmiWebClientConfigurationSpec.groovy
+++ /dev/null
@@ -1,64 +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.api.impl.config
-
-import org.springframework.beans.factory.annotation.Autowired
-import org.springframework.boot.test.context.SpringBootTest
-import org.springframework.test.context.ContextConfiguration
-import org.springframework.test.context.TestPropertySource
-import org.springframework.web.reactive.function.client.WebClient
-import spock.lang.Specification
-
-@SpringBootTest
-@ContextConfiguration(classes = [DmiWebClientConfiguration.DmiProperties])
-@TestPropertySource(properties = ["ncmp.dmi.httpclient.connectionTimeoutInSeconds=1", "ncmp.dmi.httpclient.maximumInMemorySizeInMegabytes=1"])
-class DmiWebClientConfigurationSpec extends Specification {
-
-    @Autowired
-    DmiWebClientConfiguration.DmiProperties dmiProperties
-
-    def objectUnderTest = new DmiWebClientConfiguration()
-
-    def setup() {
-        objectUnderTest.connectionTimeoutInSeconds = 10
-        objectUnderTest.maximumInMemorySizeInMegabytes = 1
-        objectUnderTest.maximumConnectionsTotal = 2
-    }
-
-    def 'DMI Properties.'() {
-        expect: 'properties are set to values in test configuration yaml file'
-            dmiProperties.authUsername == 'some-user'
-            dmiProperties.authPassword == 'some-password'
-    }
-
-    def 'Web Client Configuration construction.'() {
-        expect: 'the system can create an instance'
-            new DmiWebClientConfiguration() != null
-    }
-
-    def 'Creating a WebClient instance.'() {
-        given: 'WebClient configuration invoked'
-            def webClientInstance = objectUnderTest.webClient()
-        expect: 'the system can create an instance'
-            assert webClientInstance != null
-            assert webClientInstance instanceof WebClient
-    }
-}
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/NcmpConfigurationSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/NcmpConfigurationSpec.groovy
index f588e2ed2..74e342405 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/NcmpConfigurationSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/NcmpConfigurationSpec.groovy
@@ -31,10 +31,13 @@ import org.springframework.web.client.RestTemplate
 import spock.lang.Specification
 
 @SpringBootTest
-@ContextConfiguration(classes = [ HttpClientConfiguration])
+@ContextConfiguration(classes = [NcmpConfiguration.DmiProperties, HttpClientConfiguration])
 class NcmpConfigurationSpec extends Specification{
 
     @Autowired
+    NcmpConfiguration.DmiProperties dmiProperties
+
+    @Autowired
     HttpClientConfiguration httpClientConfiguration
 
     def mockRestTemplateBuilder = new RestTemplateBuilder()
@@ -44,6 +47,12 @@ class NcmpConfigurationSpec extends Specification{
              new NcmpConfiguration() != null
     }
 
+    def 'DMI Properties.'() {
+        expect: 'properties are set to values in test configuration yaml file'
+            dmiProperties.authUsername == 'some-user'
+            dmiProperties.authPassword == 'some-password'
+    }
+
     def 'Rest Template creation with CloseableHttpClient and MappingJackson2HttpMessageConverter.'() {
         when: 'a rest template is created'
             def result = NcmpConfiguration.restTemplate(mockRestTemplateBuilder, httpClientConfiguration)
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiDataOperationsSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiDataOperationsSpec.groovy
index e2062bc80..eb6c7a0f4 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiDataOperationsSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiDataOperationsSpec.groovy
@@ -23,7 +23,7 @@ package org.onap.cps.ncmp.api.impl.operations
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import org.onap.cps.events.EventsPublisher
-import org.onap.cps.ncmp.api.impl.config.DmiWebClientConfiguration
+import org.onap.cps.ncmp.api.impl.config.NcmpConfiguration
 import org.onap.cps.ncmp.api.impl.exception.HttpClientRequestException
 import org.onap.cps.ncmp.api.impl.utils.DmiServiceUrlBuilder
 import org.onap.cps.ncmp.api.impl.utils.context.CpsApplicationContext
@@ -52,7 +52,7 @@ import static org.onap.cps.ncmp.api.impl.operations.OperationType.READ
 import static org.onap.cps.ncmp.api.impl.operations.OperationType.UPDATE
 
 @SpringBootTest
-@ContextConfiguration(classes = [EventsPublisher, CpsApplicationContext, DmiWebClientConfiguration.DmiProperties, DmiDataOperations])
+@ContextConfiguration(classes = [EventsPublisher, CpsApplicationContext, NcmpConfiguration.DmiProperties, DmiDataOperations])
 class DmiDataOperationsSpec extends DmiOperationsBaseSpec {
 
     @SpringBean
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiModelOperationsSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiModelOperationsSpec.groovy
index a2ec9d1f0..e99e8a3d0 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiModelOperationsSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiModelOperationsSpec.groovy
@@ -23,7 +23,7 @@ package org.onap.cps.ncmp.api.impl.operations
 
 import com.fasterxml.jackson.core.JsonProcessingException
 import com.fasterxml.jackson.databind.ObjectMapper
-import org.onap.cps.ncmp.api.impl.config.DmiWebClientConfiguration
+import org.onap.cps.ncmp.api.impl.config.NcmpConfiguration
 import org.onap.cps.spi.model.ModuleReference
 import org.onap.cps.utils.JsonObjectMapper
 import org.spockframework.spring.SpringBean
@@ -37,7 +37,7 @@ import spock.lang.Shared
 import static org.onap.cps.ncmp.api.impl.operations.OperationType.READ
 
 @SpringBootTest
-@ContextConfiguration(classes = [DmiWebClientConfiguration.DmiProperties, DmiModelOperations])
+@ContextConfiguration(classes = [NcmpConfiguration.DmiProperties, DmiModelOperations])
 class DmiModelOperationsSpec extends DmiOperationsBaseSpec {
 
     @Shared
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiOperationsBaseSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiOperationsBaseSpec.groovy
index 061878e95..b7af502de 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiOperationsBaseSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/operations/DmiOperationsBaseSpec.groovy
@@ -22,7 +22,6 @@ package org.onap.cps.ncmp.api.impl.operations
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import org.onap.cps.ncmp.api.impl.client.DmiRestClient
-import org.onap.cps.ncmp.api.impl.config.DmiWebClientConfiguration
 import org.onap.cps.ncmp.api.impl.config.NcmpConfiguration
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
 import org.onap.cps.ncmp.api.impl.utils.DmiServiceUrlBuilder
@@ -51,7 +50,7 @@ abstract class DmiOperationsBaseSpec extends Specification {
     ObjectMapper spyObjectMapper = Spy()
 
     @SpringBean
-    DmiServiceUrlBuilder dmiServiceUrlBuilder = new DmiServiceUrlBuilder(new DmiWebClientConfiguration.DmiProperties(), mockCpsValidator)
+    DmiServiceUrlBuilder dmiServiceUrlBuilder = new DmiServiceUrlBuilder(new NcmpConfiguration.DmiProperties(), mockCpsValidator)
 
     def yangModelCmHandle = new YangModelCmHandle()
     def static dmiServiceName = 'some service name'
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/DmiServiceUrlBuilderSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/DmiServiceUrlBuilderSpec.groovy
index 54e9f210d..fbf2c3d78 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/DmiServiceUrlBuilderSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/DmiServiceUrlBuilderSpec.groovy
@@ -20,13 +20,12 @@
 
 package org.onap.cps.ncmp.api.impl.utils
 
-import org.onap.cps.ncmp.api.impl.config.DmiWebClientConfiguration
-
 import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.PASSTHROUGH_RUNNING
 
 import org.onap.cps.ncmp.api.impl.operations.RequiredDmiService
 import org.onap.cps.spi.utils.CpsValidator
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
+import org.onap.cps.ncmp.api.impl.config.NcmpConfiguration
 import org.onap.cps.ncmp.api.models.NcmpServiceCmHandle
 import spock.lang.Specification
 
@@ -35,7 +34,7 @@ class DmiServiceUrlBuilderSpec extends Specification {
     static YangModelCmHandle yangModelCmHandle = YangModelCmHandle.toYangModelCmHandle('dmiServiceName',
         'dmiDataServiceName', 'dmiModuleServiceName', new NcmpServiceCmHandle(cmHandleId: 'some-cm-handle-id'),'my-module-set-tag', 'my-alternate-id', 'my-data-producer-identifier')
 
-    DmiWebClientConfiguration.DmiProperties dmiProperties = new DmiWebClientConfiguration.DmiProperties()
+    NcmpConfiguration.DmiProperties dmiProperties = new NcmpConfiguration.DmiProperties()
 
     def mockCpsValidator = Mock(CpsValidator)
 
@@ -86,7 +85,7 @@ class DmiServiceUrlBuilderSpec extends Specification {
         when: 'a URL is created'
             def result = objectUnderTest.getDataOperationRequestUrl(batchRequestQueryParams, batchRequestUriVariables)
         then: 'it is formed correctly'
-            assert result.toString() == 'some-service/testBase/v1/data?topic=some+topic&requestId=some+id'
+            assert result.toString() == 'some-service/testBase/v1/data?topic=some topic&requestId=some id'
     }
 
     def 'Populate batch uri variables.'() {