From 2924fa7d806435a3bf9f2cb5abcdb01aa7295b00 Mon Sep 17 00:00:00 2001 From: Pamela Dragosh Date: Wed, 11 Mar 2020 14:06:42 -0400 Subject: 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 --- .../application/guard/CoordinationGuardTranslator.java | 5 ++--- .../pdp/application/guard/GuardPolicyRequest.java | 18 ++++++++++-------- .../xacml/pdp/application/guard/GuardTranslator.java | 8 ++------ .../xacml/pdp/application/guard/CoordinationTest.java | 4 +++- .../pdp/application/guard/GuardPolicyRequestTest.java | 10 +++++++++- 5 files changed, 26 insertions(+), 19 deletions(-) (limited to 'applications/guard') 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)); } } -- cgit 1.2.3-korg