From 57e705469481f446aeada858c8eb411c5cccebc8 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Wed, 30 Sep 2020 11:54:21 -0400 Subject: Fix new sonars in xacml-pdp Addressed the following sonar issues: - extract common test method - too many assertions in a test method Issue-ID: POLICY-2681 Change-Id: I7438be8286d52cd8479c66542fc785b27448972b Signed-off-by: Jim Hahn --- .../pdp/application/guard/GuardTranslatorTest.java | 101 +++++------------ .../OptimizationPdpApplicationTest.java | 124 ++++++--------------- 2 files changed, 62 insertions(+), 163 deletions(-) diff --git a/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/GuardTranslatorTest.java b/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/GuardTranslatorTest.java index efe698eb..7e5e3ed5 100644 --- a/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/GuardTranslatorTest.java +++ b/applications/guard/src/test/java/org/onap/policy/xacml/pdp/application/guard/GuardTranslatorTest.java @@ -34,6 +34,7 @@ import com.att.research.xacml.std.StdStatus; import com.att.research.xacml.std.StdStatusCode; import com.att.research.xacml.util.XACMLPolicyWriter; import java.io.ByteArrayOutputStream; +import java.util.HashMap; import java.util.Map; import oasis.names.tc.xacml._3_0.core.schema.wd_17.AllOfType; import oasis.names.tc.xacml._3_0.core.schema.wd_17.AnyOfType; @@ -109,83 +110,41 @@ public class GuardTranslatorTest { // JpaToscaServiceTemplate jtst = new JpaToscaServiceTemplate(); jtst.fromAuthorative(serviceTemplate); - ToscaServiceTemplate completedJtst = jtst.toAuthorative(); + final ToscaServiceTemplate completedJtst = jtst.toAuthorative(); + // + // Expected message for given policy name + // + final Map name2message = new HashMap<>(); + name2message.put("frequency-missing-properties", "Missing property limit"); + name2message.put("frequency-timewindow", "timeWindow is not an integer"); + name2message.put("minmax-notarget", "Missing target field in minmax policy"); + name2message.put("minmax-nominmax", "Missing min or max field in minmax policy"); + name2message.put("blacklist-noblacklist", "Missing blacklist"); + name2message.put("filter-noalgorithm", "Missing algorithm"); + name2message.put("filter-badalgorithm", + "Unexpected value for algorithm, should be whitelist-overrides or blacklist-overrides"); + name2message.put("filter-nofilter", "Missing filters"); + name2message.put("filter-nocollection", "Filters is not a collection"); + name2message.put("filter-noarray", "Filters is not a collection"); + name2message.put("filter-missingfield", "Missing \'field\' from filter"); + name2message.put("filter-badfield", "Unexpected value for field in filter"); + name2message.put("filter-missingfilter", "Missing \'filter\' from filter"); + name2message.put("filter-missingfunction", "Missing \'function\' from filter"); + name2message.put("filter-badfunction", "Unexpected value for function in filter"); + name2message.put("filter-missingblacklist", "Missing \'blacklist\' from filter"); + name2message.put("filter-badblacklist", "Unexpected value for blacklist in filter"); // // Get the policies // for (Map policies : completedJtst.getToscaTopologyTemplate().getPolicies()) { for (ToscaPolicy policy : policies.values()) { LOGGER.info("Testing policy " + policy.getName()); - if ("frequency-missing-properties".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Missing property limit"); - } else if ("frequency-timewindow".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("timeWindow is not an integer"); - } else if ("minmax-notarget".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Missing target field in minmax policy"); - } else if ("minmax-nominmax".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Missing min or max field in minmax policy"); - } else if ("blacklist-noblacklist".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Missing blacklist"); - } else if ("filter-noalgorithm".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Missing algorithm"); - } else if ("filter-badalgorithm".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class) - .isThrownBy(() -> translator.convertPolicy(policy)) - .withMessageContaining( - "Unexpected value for algorithm, should be whitelist-overrides or blacklist-overrides"); - } else if ("filter-nofilter".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class) - .isThrownBy(() -> translator.convertPolicy(policy)) - .withMessageContaining("Missing filters"); - } else if ("filter-nocollection".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Filters is not a collection"); - } else if ("filter-noarray".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Filters is not a collection"); - } else if ("filter-missingfield".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Missing \'field\' from filter"); - } else if ("filter-badfield".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Unexpected value for field in filter"); - } else if ("filter-missingfilter".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Missing \'filter\' from filter"); - } else if ("filter-missingfunction".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Missing \'function\' from filter"); - } else if ("filter-badfunction".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Unexpected value for function in filter"); - } else if ("filter-missingblacklist".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Missing \'blacklist\' from filter"); - } else if ("filter-badblacklist".equals(policy.getName())) { - assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> - translator.convertPolicy(policy) - ).withMessageContaining("Unexpected value for blacklist in filter"); - } + String expectedMsg = name2message.get(policy.getName()); + assertThat(expectedMsg).as(policy.getName()).isNotNull(); + + assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() -> + translator.convertPolicy(policy) + ).withMessageContaining(expectedMsg); } } } diff --git a/applications/optimization/src/test/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTest.java b/applications/optimization/src/test/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTest.java index 862f75a6..1a1e7566 100644 --- a/applications/optimization/src/test/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTest.java +++ b/applications/optimization/src/test/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTest.java @@ -224,17 +224,8 @@ public class OptimizationPdpApplicationTest { List loadedPolicies = TestUtils.loadPolicies("src/test/resources/test-optimization-policies.yaml", service); assertThat(loadedPolicies).isNotNull().hasSize(14); - // - // Ask for a decision for available default policies - // - DecisionResponse response = makeDecision(); - assertThat(response).isNotNull(); - assertThat(response.getPolicies()).hasSize(2); - // - // Validate it - // - validateDecision(response, baseRequest); + validateDecisionCount(2); } /** @@ -279,16 +270,8 @@ public class OptimizationPdpApplicationTest { // Add US to the geography list // ((List) baseRequest.getResource().get("geography")).add("US"); - // - // Ask for a decision for default US Policy - // - DecisionResponse response = makeDecision(); - assertThat(response).isNotNull(); - assertThat(response.getPolicies()).hasSize(2); - // - // Validate it - // - validateDecision(response, baseRequest); + + validateDecisionCount(2); } /** @@ -301,17 +284,8 @@ public class OptimizationPdpApplicationTest { // Add vCPE to the service list // ((List) baseRequest.getResource().get("services")).add("vCPE"); - // - // Ask for a decision for default US policy for vCPE service - // - DecisionResponse response = makeDecision(); - assertThat(response).isNotNull(); - assertThat(response.getPolicies()).hasSize(3); - // - // Validate it - // - validateDecision(response, baseRequest); + validateDecisionCount(3); } /** @@ -324,17 +298,8 @@ public class OptimizationPdpApplicationTest { // Add vG to the resource list // ((List) baseRequest.getResource().get("resources")).add("vG"); - // - // Ask for a decision for default US service vCPE resource vG policy - // - DecisionResponse response = makeDecision(); - assertThat(response).isNotNull(); - assertThat(response.getPolicies()).hasSize(6); - // - // Validate it - // - validateDecision(response, baseRequest); + validateDecisionCount(6); } /** @@ -347,18 +312,8 @@ public class OptimizationPdpApplicationTest { // Add gold as a scope // ((List) baseRequest.getContext().get("subscriberName")).add("subscriber_a"); - // - // Ask for a decision for specific US vCPE vG gold - // - DecisionResponse response = makeDecision(); - assertThat(response).isNotNull(); - assertThat(response.getPolicies()).hasSize(6); - assertThat(response.getAdvice()).hasSize(2); - // - // Validate it - // - validateDecision(response, baseRequest); + validateDecisionCount(6, 2); } /** @@ -372,18 +327,8 @@ public class OptimizationPdpApplicationTest { // ((List) baseRequest.getResource().get("scope")).remove("gold"); ((List) baseRequest.getContext().get("subscriberName")).add("subscriber_x"); - // - // Ask for a decision for specific US vCPE vG (gold or platinum) - // - DecisionResponse response = makeDecision(); - assertThat(response).isNotNull(); - assertThat(response.getPolicies()).hasSize(8); - assertThat(response.getAdvice()).hasSize(2); - // - // Validate it - // - validateDecision(response, baseRequest); + validateDecisionCount(8, 2); } /** @@ -398,17 +343,8 @@ public class OptimizationPdpApplicationTest { ((List) baseRequest.getResource().get("scope")).remove("gold"); ((List) baseRequest.getResource().get("scope")).remove("platinum"); ((List) baseRequest.getContext().get("subscriberName")).remove("subscriber_a"); - // - // Ask for a decision for specific US vCPE vG gold - // - DecisionResponse response = makeDecision(); - assertThat(response).isNotNull(); - assertThat(response.getPolicies()).hasSize(7); - // - // Validate it - // - validateDecision(response, baseRequest); + validateDecisionCount(7); } /** @@ -421,17 +357,8 @@ public class OptimizationPdpApplicationTest { // List policyTypes = Lists.newArrayList("onap.policies.optimization.resource.AffinityPolicy"); baseRequest.getResource().put("policy-type", policyTypes); - // - // Ask for a decision for default - // - DecisionResponse response = makeDecision(); - assertThat(response).isNotNull(); - assertThat(response.getPolicies()).hasSize(1); - // - // Validate it - // - validateDecision(response, baseRequest); + validateDecisionCount(1); } /** @@ -445,17 +372,8 @@ public class OptimizationPdpApplicationTest { // ((List) baseRequest.getResource().get("policy-type")) .add("onap.policies.optimization.resource.HpaPolicy"); - // - // Ask for a decision for default - // - DecisionResponse response = makeDecision(); - assertThat(response).isNotNull(); - assertThat(response.getPolicies()).hasSize(2); - // - // Validate it - // - validateDecision(response, baseRequest); + validateDecisionCount(2); } @Test @@ -504,6 +422,28 @@ public class OptimizationPdpApplicationTest { return decision.getKey(); } + private DecisionResponse validateDecisionCount(int expectedPolicyCount) { + // + // Ask for a decision for default + // + DecisionResponse response = makeDecision(); + + assertThat(response).isNotNull(); + assertThat(response.getPolicies()).hasSize(expectedPolicyCount); + // + // Validate it + // + validateDecision(response, baseRequest); + + return response; + } + + private void validateDecisionCount(int expectedPolicyCount, int expectedAdviceCount) { + DecisionResponse response = validateDecisionCount(expectedPolicyCount); + + assertThat(response.getAdvice()).hasSize(expectedAdviceCount); + } + @SuppressWarnings("unchecked") private void validateDecision(DecisionResponse decision, DecisionRequest request) { for (Entry entrySet : decision.getPolicies().entrySet()) { -- cgit 1.2.3-korg