Refactor: Improve PolicyExecutor WebClient error logging

- Improved logging and ensured consistency with processFallbackResponse().

Issue-ID: CPS-2859
Change-Id: Ia74226e0aab79bc0542eff6869ccbcbd3995276d
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>

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 3810532..6e69a5c 100644 (file)
--- 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
@@ -46,6 +46,7 @@ import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Service;
 import org.springframework.web.reactive.function.BodyInserters;
 import org.springframework.web.reactive.function.client.WebClient;
+import org.springframework.web.reactive.function.client.WebClientRequestException;
 import org.springframework.web.reactive.function.client.WebClientResponseException;
 
 @Slf4j
@@ -155,6 +156,10 @@ public class PolicyExecutor {
 
         final Object bodyAsObject = createBodyAsObject(operationAsMap);
 
+        log.debug("Sending permission check to Policy Executor for CMHandle: {} with operation: {}",
+                yangModelCmHandle.getId(), operationType);
+        log.trace("Policy Executor request body: {}", bodyAsObject);
+
         final UrlTemplateParameters urlTemplateParameters = RestServiceUrlTemplateBuilder.newInstance()
                 .fixedPathSegment(REQUEST_PATH)
                 .createUrlTemplateParameters(String.format("%s:%s", serverAddress, serverPort), PERMISSION_BASE_PATH);
@@ -191,26 +196,41 @@ public class PolicyExecutor {
         }
     }
 
+    private String createErrorDetailsMessage(final Throwable throwable) {
+        if (throwable instanceof WebClientResponseException webClientResponseException) {
+            return "Policy Executor returned HTTP Status code "
+                    + webClientResponseException.getStatusCode().value() + ".";
+        }
+        if (throwable instanceof WebClientRequestException) {
+            return "Network or I/O error while attempting to contact Policy Executor.";
+        }
+        if (throwable instanceof TimeoutException || throwable.getCause() instanceof TimeoutException) {
+            return "Policy Executor request timed out.";
+        }
+        if (throwable.getCause() instanceof UnknownHostException) {
+            return String.format("Cannot connect to Policy Executor (%s:%s).", serverAddress, serverPort);
+        }
+        return "Unexpected error during Policy Executor call.";
+    }
+
     private void processException(final RuntimeException runtimeException) {
-        if (runtimeException instanceof WebClientResponseException) {
-            final WebClientResponseException webClientResponseException = (WebClientResponseException) runtimeException;
-            log.warn("HTTP Error Message: {}", webClientResponseException.getMessage());
-            final int httpStatusCode = webClientResponseException.getStatusCode().value();
-            processFallbackResponse("Policy Executor returned HTTP Status code " + httpStatusCode + ".",
-                webClientResponseException);
-        } else {
-            final Throwable cause = runtimeException.getCause();
-            if (cause instanceof TimeoutException) {
-                processFallbackResponse("Policy Executor request timed out.", cause);
-            } else if (cause instanceof UnknownHostException) {
-                final String message
-                    = String.format("Cannot connect to Policy Executor (%s:%s).", serverAddress, serverPort);
-                processFallbackResponse(message, cause);
-            } else {
-                log.warn("Request to Policy Executor failed with unexpected exception", runtimeException);
-                throw runtimeException;
-            }
+        final String errorDetailsMessage = createErrorDetailsMessage(runtimeException);
+
+        log.warn("Exception during Policy Execution check. Class: {}, Message: {}, Details: {}",
+                runtimeException.getClass().getSimpleName(), runtimeException.getMessage(), errorDetailsMessage);
+
+        if (runtimeException instanceof WebClientResponseException
+                || runtimeException instanceof WebClientRequestException) {
+            processFallbackResponse(errorDetailsMessage, runtimeException);
+            return;
+        }
+        final Throwable nestedThrowable = runtimeException.getCause();
+        if (nestedThrowable instanceof TimeoutException || nestedThrowable instanceof UnknownHostException) {
+            final String nestedErrorDetailsMessage = createErrorDetailsMessage(nestedThrowable);
+            processFallbackResponse(nestedErrorDetailsMessage, nestedThrowable);
+            return;
         }
+        throw runtimeException;
     }
 
     private void processFallbackResponse(final String message, final Throwable cause) {
@@ -220,5 +240,4 @@ public class PolicyExecutor {
         log.warn(warning);
         processDecision(decisionId, decision, warning, cause);
     }
-
 }

diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy
index 9423246..96330ab 100644 (file)
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy
@@ -1,6 +1,6 @@
 /*
  *  ============LICENSE_START=======================================================
- *  Copyright (C) 2024 Nordix Foundation
+ *  Copyright (C) 2024-2025 OpenInfra Foundation Europe. All rights reserved.
  *  ================================================================================
  *  Licensed under the Apache License, Version 2.0 (the "License");
  *  you may not use this file except in compliance with the License.
@@ -33,10 +33,10 @@ import org.slf4j.LoggerFactory
 import org.springframework.http.HttpStatus
 import org.springframework.http.ResponseEntity
 import org.springframework.web.reactive.function.client.WebClient
+import org.springframework.web.reactive.function.client.WebClientRequestException
 import org.springframework.web.reactive.function.client.WebClientResponseException
 import reactor.core.publisher.Mono
 import spock.lang.Specification
-
 import java.util.concurrent.TimeoutException
 
 import static org.onap.cps.ncmp.api.data.models.OperationType.CREATE
@@ -79,7 +79,7 @@ class PolicyExecutorSpec extends Specification {
         when: 'permission is checked for an operation'
             objectUnderTest.checkPermission(new YangModelCmHandle(), operationType, 'my credentials','my resource',someValidJson)
         then: 'system logs the operation is allowed'
-            assert getLogEntry(2) == 'Operation allowed.'
+            assert getLogEntry(4) == 'Operation allowed.'
         and: 'no exception occurs'
             noExceptionThrown()
         where: 'all write operations are tested'
@@ -129,7 +129,7 @@ class PolicyExecutorSpec extends Specification {
         when: 'permission is checked for an operation'
             objectUnderTest.checkPermission(new YangModelCmHandle(), CREATE, 'my credentials', 'my resource', someValidJson)
         then: 'system logs the expected message'
-            assert getLogEntry(1) == 'No valid response body from Policy Executor, ignored'
+            assert getLogEntry(3) == 'No valid response body from Policy Executor, ignored'
     }
 
     def 'Permission check with timeout exception.'() {
@@ -159,7 +159,7 @@ class PolicyExecutorSpec extends Specification {
         then: 'Policy Executor exception is thrown'
             def thrownException = thrown(PolicyExecutorException)
             assert thrownException.message == 'Operation not allowed. Decision id N/A : deny by default'
-            assert thrownException.details == 'Cannot connect to Policy Executor (some host:some port). Falling back to configured default decision: deny by default'
+            assert thrownException.details == 'Unexpected error during Policy Executor call. Falling back to configured default decision: deny by default'
         and: 'the cause is the original unknown host exception'
             assert thrownException.cause == unknownHostException
     }
@@ -208,6 +208,21 @@ class PolicyExecutorSpec extends Specification {
             assert getLogEntry(0) == 'Policy Executor Enabled: false'
     }
 
+    def 'Permission check with web client request exception.'() {
+        given: 'a WebClientRequestException is thrown during the Policy Executor call'
+            def webClientRequestException = Mock(WebClientRequestException)
+            webClientRequestException.getMessage() >> "some error message"
+            mockResponseSpec.toEntity(*_) >> { throw webClientRequestException }
+            objectUnderTest.defaultDecision = 'deny'
+        when: 'permission is checked for a write operation'
+            objectUnderTest.checkPermission(new YangModelCmHandle(), CREATE, 'my credentials', 'my resource', someValidJson)
+        then: 'a PolicyExecutorException is thrown with the expected fallback message'
+            def thrownException = thrown(PolicyExecutorException)
+            thrownException.message == 'Operation not allowed. Decision id N/A : deny'
+            thrownException.details == 'Network or I/O error while attempting to contact Policy Executor. Falling back to configured default decision: deny'
+            thrownException.cause == webClientRequestException
+    }
+
     def mockResponse(mockResponseAsMap, httpStatus) {
         JsonNode jsonNode = spiedObjectMapper.readTree(spiedObjectMapper.writeValueAsString(mockResponseAsMap))
         def mono = Mono.just(new ResponseEntity<>(jsonNode, httpStatus))