Policy Executor: handle errors, part 2

(fighting between IntelliJ and Checkstyle best practices)

- non-2xx responses are processed using web client exceptions
- handle unknown host exception
- upgraded spotbugs (checkstyle and related mvn plugin)
- fixed some small spotbugs due to upgrade
- added commented instructions in docker compose to enable debugging
- added some environment variables for policy executor configuration
- extract out Sleeper in stub service to achieve 100% coverage
- added cause to Policy Executor exceptions where applicable
- ignored (new) spotbug rule about catch NPE because of issue in 3pp
- ignored (new) spotbug rule about \n in string due to multiline string block

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

4 files changed, 66 insertions, 45 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/NcmpException.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/NcmpException.java
index 6754965866..3c81d0f536 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/NcmpException.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/NcmpException.java
@@ -39,7 +39,18 @@ public class NcmpException extends RuntimeException {
      * @param details the error details
      */
     public NcmpException(final String message, final String details) {
-        super(message);
+        this(message, details, null);
+    }
+
+    /**
+     * Constructor with cause.
+     *
+     * @param message the error message
+     * @param details the error details
+     * @param cause the cause of the exception
+     */
+    public NcmpException(final String message, final String details, final Throwable cause) {
+        super(message, cause);
         this.details = details;
     }
 
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/PolicyExecutorException.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/PolicyExecutorException.java
index 333c12271b..bb753b85f1 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/PolicyExecutorException.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/PolicyExecutorException.java
@@ -35,8 +35,9 @@ public class PolicyExecutorException extends NcmpException {
      *
      * @param message response message
      * @param details response details
+     * @param cause   the cause of the exception
      */
-    public PolicyExecutorException(final String message, final String details) {
-        super(message, details);
+    public PolicyExecutorException(final String message, final String details, final Throwable cause) {
+        super(message, details, cause);
     }
 }
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 caed28a648..af4331893d 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
@@ -23,6 +23,7 @@ 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.net.UnknownHostException;
 import java.time.Duration;
 import java.time.temporal.ChronoUnit;
 import java.util.Collections;
@@ -45,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.WebClientResponseException;
 
 @Slf4j
 @Service
@@ -71,6 +73,8 @@ public class PolicyExecutor {
 
     private final ObjectMapper objectMapper;
 
+    private static final Throwable NO_ERROR = null;
+
     /**
      * 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)
@@ -88,26 +92,20 @@ public class PolicyExecutor {
                                 final String changeRequestAsJson) {
         log.trace("Policy Executor Enabled: {}", enabled);
         if (enabled) {
-            ResponseEntity<JsonNode> responseEntity = null;
             try {
-                responseEntity =
-                    getPolicyExecutorResponse(yangModelCmHandle, operationType, authorization, resourceIdentifier,
-                        changeRequestAsJson);
-            } catch (final RuntimeException runtimeException) {
-                processException(runtimeException);
-            }
-            if (responseEntity == null) {
-                log.warn("No valid response from Policy Executor, ignored");
-                return;
-            }
-            if (responseEntity.getStatusCode().is2xxSuccessful()) {
-                if (responseEntity.getBody() == null) {
+                final ResponseEntity<JsonNode> responseEntity = getPolicyExecutorResponse(yangModelCmHandle,
+                                                                                          operationType,
+                                                                                          authorization,
+                                                                                          resourceIdentifier,
+                                                                                          changeRequestAsJson);
+                final JsonNode responseBody = responseEntity.getBody();
+                if (responseBody == null) {
                     log.warn("No valid response body from Policy Executor, ignored");
                     return;
                 }
-                processSuccessResponse(responseEntity.getBody());
-            } else {
-                processNon2xxResponse(responseEntity.getStatusCode().value());
+                processSuccessResponse(responseBody);
+            } catch (final RuntimeException runtimeException) {
+                processException(runtimeException);
             }
         }
     }
@@ -173,35 +171,17 @@ public class PolicyExecutor {
             .block();
     }
 
-    private void processNon2xxResponse(final int httpStatusCode) {
-        processFallbackResponse("Policy Executor returned HTTP Status code " + httpStatusCode + ".");
-    }
-
-    private void processException(final RuntimeException runtimeException) {
-        if (runtimeException.getCause() instanceof TimeoutException) {
-            processFallbackResponse("Policy Executor request timed out.");
-        } else {
-            log.warn("Request to Policy Executor failed with unexpected exception", runtimeException);
-            throw runtimeException;
-        }
-    }
-
-    private void processFallbackResponse(final String message) {
-        final String decisionId = "N/A";
-        final String decision = defaultDecision;
-        final String warning = message + " Falling back to configured default decision: " + defaultDecision;
-        log.warn(warning);
-        processDecision(decisionId, decision, warning);
-    }
-
     private static void processSuccessResponse(final JsonNode responseBody) {
         final String decisionId = responseBody.path("decisionId").asText("unknown id");
         final String decision = responseBody.path("decision").asText("unknown");
         final String messageFromPolicyExecutor = responseBody.path("message").asText();
-        processDecision(decisionId, decision, messageFromPolicyExecutor);
+        processDecision(decisionId, decision, messageFromPolicyExecutor, NO_ERROR);
     }
 
-    private static void processDecision(final String decisionId, final String decision, final String details) {
+    private static void processDecision(final String decisionId,
+                                        final String decision,
+                                        final String details,
+                                        final Throwable optionalCauseOfError) {
         log.trace("Policy Executor decision id: {} ", decisionId);
         if ("allow".equals(decision)) {
             log.trace("Operation allowed.");
@@ -209,8 +189,37 @@ public class PolicyExecutor {
             log.warn("Policy Executor decision: {}", decision);
             log.warn("Policy Executor message: {}", details);
             final String message = "Operation not allowed. Decision id " + decisionId + " : " + decision;
-            throw new PolicyExecutorException(message, details);
+            throw new PolicyExecutorException(message, details, optionalCauseOfError);
         }
     }
 
+    private void processException(final RuntimeException runtimeException) {
+        if (runtimeException instanceof WebClientResponseException) {
+            final WebClientResponseException webClientResponseException = (WebClientResponseException) runtimeException;
+            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;
+            }
+        }
+    }
+
+    private void processFallbackResponse(final String message, final Throwable cause) {
+        final String decisionId = "N/A";
+        final String decision = defaultDecision;
+        final String warning = message + " Falling back to configured default decision: " + defaultDecision;
+        log.warn(warning);
+        processDecision(decisionId, decision, warning, cause);
+    }
+
 }
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/lcm/LcmEventType.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/lcm/LcmEventType.java
index 4bc2f10218..1d4e3c8363 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/lcm/LcmEventType.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/lcm/LcmEventType.java
@@ -26,10 +26,10 @@ public enum LcmEventType {
 
     private final String eventName;
 
-    private final String eventTypeTemplate = "org.onap.ncmp.cmhandle-lcm-event.%s";
+    private static final String EVENT_TYPE_TEMPLATE = "org.onap.ncmp.cmhandle-lcm-event.%s";
 
     LcmEventType(final String eventName) {
-        this.eventName = String.format(eventTypeTemplate, eventName);
+        this.eventName = String.format(EVENT_TYPE_TEMPLATE, eventName);
     }
 
     public String getEventType() {