Better error handling for decisions

Throw exceptions when requests cannot be created and return
error information back. Consolidated some code to avoid sonar
duplication issues.

Companion review to https://gerrit.onap.org/r/c/policy/models/+/103548

Issue-ID: POLICY-2242
Change-Id: Ic873af933dab82e3aeef6335f55939666be20385
Signed-off-by: Pamela Dragosh <pdragosh@research.att.com>

5 files changed, 26 insertions, 19 deletions

diff --git a/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/CoordinationGuardTranslator.java b/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/CoordinationGuardTranslator.java
index 60fac498..f1006c6d 100644
--- a/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/CoordinationGuardTranslator.java
+++ b/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/CoordinationGuardTranslator.java
@@ -99,9 +99,8 @@ public class CoordinationGuardTranslator implements ToscaPolicyTranslator {
      * the one in LegacyGuardTranslator is used.
      */
     @Override
-    public Request convertRequest(DecisionRequest request) {
-        LOGGER.info("this convertRequest shouldn't be used");
-        return null;
+    public Request convertRequest(DecisionRequest request) throws ToscaPolicyConversionException {
+        throw new ToscaPolicyConversionException("this convertRequest shouldn't be used");
     }
 
     /**
diff --git a/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/GuardPolicyRequest.java b/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/GuardPolicyRequest.java
index 51d8f0ed..1e3e9150 100644
--- a/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/GuardPolicyRequest.java
+++ b/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/GuardPolicyRequest.java
@@ -34,13 +34,13 @@ import lombok.Setter;
 import lombok.ToString;
 
 import org.onap.policy.models.decisions.concepts.DecisionRequest;
+import org.onap.policy.pdp.xacml.application.common.ToscaPolicyConversionException;
 
 @Getter
 @Setter
 @ToString
 @XACMLRequest(ReturnPolicyIdList = true)
 public class GuardPolicyRequest {
-
     private static final String STR_GUARD = "guard";
 
     @XACMLSubject(includeInResults = true)
@@ -82,9 +82,11 @@ public class GuardPolicyRequest {
      *
      * @param decisionRequest Input DecisionRequest
      * @return StdMetadataPolicyRequest
+     * @throws ToscaPolicyConversionException If we cannot parse the request
      */
     @SuppressWarnings("unchecked")
-    public static GuardPolicyRequest createInstance(DecisionRequest decisionRequest) {
+    public static GuardPolicyRequest createInstance(DecisionRequest decisionRequest) 
+            throws ToscaPolicyConversionException {
         //
         // Create our return object
         //
@@ -133,14 +135,14 @@ public class GuardPolicyRequest {
             request.targetId = guard.get("target").toString();
         }
         if (guard.containsKey("vfCount")) {
-            //
-            // TODO this can potentially throw a NumberFormatException. Fix this to
-            // throw the exception when you fix the ConvertRequest to throw exceptions also.
-            //
-            request.vfCount = Integer.decode(guard.get("vfCount").toString());
+            try {
+                request.vfCount = Integer.decode(guard.get("vfCount").toString());
+            } catch (NumberFormatException e) {
+                throw new ToscaPolicyConversionException("Failed to decode vfCount", e);
+            }
         }
 
         return request;
     }
 
-}
+}
\ No newline at end of file
diff --git a/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/GuardTranslator.java b/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/GuardTranslator.java
index 4365cad6..fd46a988 100644
--- a/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/GuardTranslator.java
+++ b/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/GuardTranslator.java
@@ -140,17 +140,13 @@ public class GuardTranslator implements ToscaPolicyTranslator {
     /**
      * Convert Request.
      */
-    public Request convertRequest(DecisionRequest request) {
+    public Request convertRequest(DecisionRequest request) throws ToscaPolicyConversionException {
         LOGGER.info("Converting Request {}", request);
         try {
             return RequestParser.parseRequest(GuardPolicyRequest.createInstance(request));
         } catch (IllegalArgumentException | IllegalAccessException | DataTypeException e) {
-            LOGGER.error("Failed to convert DecisionRequest", e);
+            throw new ToscaPolicyConversionException("Failed to convert DecisionRequest", e);
         }
-        //
-        // TODO throw exception
-        //
-        return null;
     }
 
     /**
diff --git a/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/CoordinationTest.java b/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/CoordinationTest.java
index b5585a7c..5b62f364 100644
--- a/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/CoordinationTest.java
+++ b/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/CoordinationTest.java
@@ -23,6 +23,7 @@
 package org.onap.policy.xacml.pdp.application.guard;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 import com.att.research.xacml.api.Response;
 import java.io.File;
@@ -191,7 +192,8 @@ public class CoordinationTest {
         // the application.
         //
         CoordinationGuardTranslator translator = new CoordinationGuardTranslator();
-        assertThat(translator.convertRequest(null)).isNull();
+        assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() ->
+            translator.convertRequest(null)).withMessage("this convertRequest shouldn't be used");
         assertThat(translator.convertResponse(null)).isNull();
         assertThat(CoordinationGuardTranslator.loadCoordinationDirectiveFromFile(
                 policyFolder.getRoot().getAbsolutePath() + "/nonexist.yaml")).isNull();
diff --git a/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/GuardPolicyRequestTest.java b/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/GuardPolicyRequestTest.java
index b3ef3bdd..41fd4705 100644
--- a/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/GuardPolicyRequestTest.java
+++ b/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/GuardPolicyRequestTest.java
@@ -23,16 +23,18 @@
 package org.onap.policy.xacml.pdp.application.guard;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.Test;
 import org.onap.policy.models.decisions.concepts.DecisionRequest;
+import org.onap.policy.pdp.xacml.application.common.ToscaPolicyConversionException;
 
 public class GuardPolicyRequestTest {
 
     @Test
-    public void testAnomalies() {
+    public void testAnomalies() throws ToscaPolicyConversionException {
         DecisionRequest decisionRequest = new DecisionRequest();
         assertThat(GuardPolicyRequest.createInstance(decisionRequest)).isNotNull();
 
@@ -82,6 +84,12 @@ public class GuardPolicyRequestTest {
         resources.put("guard", guard);
         decisionRequest.setResource(resources);
         assertThat(GuardPolicyRequest.createInstance(decisionRequest)).isNotNull();
+
+        guard.put("vfCount", "I am not valid");
+        resources.put("guard", guard);
+        decisionRequest.setResource(resources);
+        assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() ->
+            GuardPolicyRequest.createInstance(decisionRequest));
     }
 
 }