Policy Executor: pass Change request as Object

- Added unit test for new exception handling

Issue-ID: CPS-2335
Change-Id: I5517c91edf505932d0a94165c19420922b3f5f51
Signed-off-by: ToineSiebelink <toine.siebelink@est.tech>
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>

3 files changed, 35 insertions, 14 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutor.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutor.java
index 89b48f3755..b3aa848394 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutor.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutor.java
@@ -20,7 +20,9 @@
 
 package org.onap.cps.ncmp.impl.data.policyexecutor;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -28,6 +30,7 @@ import java.util.Map;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.onap.cps.ncmp.api.data.models.OperationType;
+import org.onap.cps.ncmp.api.exceptions.NcmpException;
 import org.onap.cps.ncmp.api.exceptions.PolicyExecutorException;
 import org.onap.cps.ncmp.api.exceptions.ServerNcmpException;
 import org.onap.cps.ncmp.impl.inventory.models.YangModelCmHandle;
@@ -58,6 +61,8 @@ public class PolicyExecutor {
     @Qualifier("policyExecutorWebClient")
     private final WebClient policyExecutorWebClient;
 
+    private final ObjectMapper objectMapper;
+
     /**
      * Use the Policy Executor to check permission for a cm write operation.
      * Wil throw an exception when the operation is not permitted (work in progress)
@@ -108,7 +113,13 @@ public class PolicyExecutor {
         data.put("resourceIdentifier", resourceIdentifier);
         data.put("targetIdentifier", yangModelCmHandle.getAlternateId());
         if (!OperationType.DELETE.equals(operationType)) {
-            data.put("cmChangeRequest", changeRequestAsJson);
+            try {
+                final Object changeRequestAsObject = objectMapper.readValue(changeRequestAsJson, Object.class);
+                data.put("cmChangeRequest", changeRequestAsObject);
+            } catch (final JsonProcessingException e) {
+                throw new NcmpException("Cannot convert Change Request data to Object",
+                    "Invalid Json: " + changeRequestAsJson);
+            }
         }
 
         final Map<String, Object> request = new HashMap<>(2);
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/PolicyExecutorConfigurationSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorConfigurationSpec.groovy
index c086eab810..c859bb0a09 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/PolicyExecutorConfigurationSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorConfigurationSpec.groovy
@@ -18,10 +18,10 @@
  *  ============LICENSE_END=========================================================
  */
 
-package org.onap.cps.ncmp.impl.data
+package org.onap.cps.ncmp.impl.data.policyexecutor
 
+import com.fasterxml.jackson.databind.ObjectMapper
 import org.onap.cps.ncmp.config.PolicyExecutorHttpClientConfig
-import org.onap.cps.ncmp.impl.data.policyexecutor.PolicyExecutor
 import org.onap.cps.ncmp.impl.policyexecutor.PolicyExecutorWebClientConfiguration
 import org.onap.cps.ncmp.utils.WebClientBuilderTestConfig
 import org.springframework.beans.factory.annotation.Autowired
@@ -30,7 +30,7 @@ import org.springframework.test.context.ContextConfiguration
 import spock.lang.Specification
 
 @SpringBootTest
-@ContextConfiguration(classes = [PolicyExecutor, PolicyExecutorWebClientConfiguration,  PolicyExecutorHttpClientConfig, WebClientBuilderTestConfig ])
+@ContextConfiguration(classes = [ObjectMapper, PolicyExecutor, PolicyExecutorWebClientConfiguration,  PolicyExecutorHttpClientConfig, WebClientBuilderTestConfig ])
 class PolicyExecutorConfigurationSpec extends Specification {
 
     @Autowired
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/PolicyExecutorSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy
index a5776676dc..63a915ab64 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/PolicyExecutorSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy
@@ -18,7 +18,7 @@
  *  ============LICENSE_END=========================================================
  */
 
-package org.onap.cps.ncmp.impl.data
+package org.onap.cps.ncmp.impl.data.policyexecutor
 
 import ch.qos.logback.classic.Level
 import ch.qos.logback.classic.Logger
@@ -26,9 +26,9 @@ import ch.qos.logback.classic.spi.ILoggingEvent
 import ch.qos.logback.core.read.ListAppender
 import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.ObjectMapper
+import org.onap.cps.ncmp.api.exceptions.NcmpException
 import org.onap.cps.ncmp.api.exceptions.PolicyExecutorException
 import org.onap.cps.ncmp.api.exceptions.ServerNcmpException
-import org.onap.cps.ncmp.impl.data.policyexecutor.PolicyExecutor
 import org.onap.cps.ncmp.impl.inventory.models.YangModelCmHandle
 import org.slf4j.LoggerFactory
 import org.springframework.http.HttpStatus
@@ -47,12 +47,13 @@ class PolicyExecutorSpec extends Specification {
     def mockWebClient = Mock(WebClient)
     def mockRequestBodyUriSpec = Mock(WebClient.RequestBodyUriSpec)
     def mockResponseSpec = Mock(WebClient.ResponseSpec)
+    def spiedObjectMapper = Spy(ObjectMapper)
 
-    PolicyExecutor objectUnderTest = new PolicyExecutor(mockWebClient)
+    PolicyExecutor objectUnderTest = new PolicyExecutor(mockWebClient, spiedObjectMapper)
 
     def logAppender = Spy(ListAppender<ILoggingEvent>)
 
-    ObjectMapper objectMapper = new ObjectMapper()
+    def someValidJson = '{"Hello":"World"}'
 
     def setup() {
         setupLogger()
@@ -72,7 +73,7 @@ class PolicyExecutorSpec extends Specification {
         given: 'allow response'
             mockResponse([decision:'allow'], HttpStatus.OK)
         when: 'permission is checked for an operation'
-            objectUnderTest.checkPermission(new YangModelCmHandle(), operationType, 'my credentials','my resource','my change')
+            objectUnderTest.checkPermission(new YangModelCmHandle(), operationType, 'my credentials','my resource',someValidJson)
         then: 'system logs the operation is allowed'
             assert getLogEntry(2) == 'Policy Executor allows the operation'
         and: 'no exception occurs'
@@ -85,7 +86,7 @@ class PolicyExecutorSpec extends Specification {
         given: 'other response'
             mockResponse([decision:'other', decisionId:123, message:'I dont like Mondays' ], HttpStatus.OK)
         when: 'permission is checked for an operation'
-            objectUnderTest.checkPermission(new YangModelCmHandle(), PATCH, 'my credentials','my resource','my change')
+            objectUnderTest.checkPermission(new YangModelCmHandle(), PATCH, 'my credentials','my resource',someValidJson)
         then: 'Policy Executor exception is thrown'
             def thrownException = thrown(PolicyExecutorException)
             assert thrownException.message == 'Policy Executor did not allow request. Decision #123 : other'
@@ -96,7 +97,7 @@ class PolicyExecutorSpec extends Specification {
         given: 'other response'
             mockResponse([], HttpStatus.I_AM_A_TEAPOT)
         when: 'permission is checked for an operation'
-            objectUnderTest.checkPermission(new YangModelCmHandle(), PATCH, 'my credentials','my resource','my change')
+            objectUnderTest.checkPermission(new YangModelCmHandle(), PATCH, 'my credentials','my resource',someValidJson)
         then: 'Server Ncmp exception is thrown'
             def thrownException = thrown(ServerNcmpException)
             assert thrownException.message == 'Policy Executor invocation failed'
@@ -107,7 +108,7 @@ class PolicyExecutorSpec extends Specification {
         given: 'invalid response from Policy executor'
             mockResponseSpec.toEntity(*_) >> invalidResponse
         when: 'permission is checked for an operation'
-            objectUnderTest.checkPermission(new YangModelCmHandle(), CREATE, 'my credentials','my resource','my change')
+            objectUnderTest.checkPermission(new YangModelCmHandle(), CREATE, 'my credentials','my resource',someValidJson)
         then: 'system logs the expected message'
             assert getLogEntry(1) == expectedMessage
         where: 'following invalid responses are received'
@@ -116,17 +117,26 @@ class PolicyExecutorSpec extends Specification {
             Mono.just(new ResponseEntity<>(null, HttpStatus.OK))   || 'No valid response body from policy, ignored'
     }
 
+    def 'Permission check with an invalid change request json.'() {
+        when: 'permission is checked for an invalid change request'
+            objectUnderTest.checkPermission(new YangModelCmHandle(), CREATE, 'my credentials', 'my resource', 'invalid json string')
+        then: 'an ncmp exception thrown'
+            def ncmpException = thrown(NcmpException)
+            ncmpException.message == 'Cannot convert Change Request data to Object'
+            ncmpException.details.contains('invalid json string')
+    }
+
     def 'Permission check feature disabled.'() {
         given: 'feature is disabled'
             objectUnderTest.enabled = false
         when: 'permission is checked for an operation'
-            objectUnderTest.checkPermission(new YangModelCmHandle(), PATCH, 'my credentials','my resource','my change')
+            objectUnderTest.checkPermission(new YangModelCmHandle(), PATCH, 'my credentials','my resource',someValidJson)
         then: 'system logs that the feature not enabled'
             assert getLogEntry(0) == 'Policy Executor Enabled: false'
     }
 
     def mockResponse(mockResponseAsMap, httpStatus) {
-        JsonNode jsonNode = objectMapper.readTree(objectMapper.writeValueAsString(mockResponseAsMap))
+        JsonNode jsonNode = spiedObjectMapper.readTree(spiedObjectMapper.writeValueAsString(mockResponseAsMap))
         def mono = Mono.just(new ResponseEntity<>(jsonNode, httpStatus))
         mockResponseSpec.toEntity(*_) >> mono
     }