Replace validation code with annotations

Instead of having code to validate various values, created POJOs to
represent the decoded data so that bean validation annotations could be
used instead.
Didn't see any obvious ways to use annotations in the Optimization code,
but did notice a bug (passed role instead of provisions).  Extracted a
common method which fixed the bug as a side-effect.

Issue-ID: POLICY-2418
Change-Id: I9ef589086fc8f7f66810b66405fbf302d7570e5a
Signed-off-by: Jim Hahn <jrh3@att.com>

diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/ToscaPolicyTranslatorUtils.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/ToscaPolicyTranslatorUtils.java
index e19130d..e682437 100644 (file)
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/ToscaPolicyTranslatorUtils.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/ToscaPolicyTranslatorUtils.java
@@ -24,6 +24,7 @@ package org.onap.policy.pdp.xacml.application.common;
 
 import com.att.research.xacml.api.Identifier;
 import com.att.research.xacml.api.XACML3;
+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;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.ApplyType;
@@ -35,6 +36,10 @@ import oasis.names.tc.xacml._3_0.core.schema.wd_17.ObjectFactory;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.TargetType;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.VariableReferenceType;
 import org.apache.commons.lang3.StringUtils;
+import org.onap.policy.common.parameters.BeanValidationResult;
+import org.onap.policy.common.parameters.BeanValidator;
+import org.onap.policy.common.utils.coder.CoderException;
+import org.onap.policy.common.utils.coder.StandardCoder;
 
 /**
  * This class contains static methods of helper classes to convert TOSCA policies
@@ -45,6 +50,7 @@ import org.apache.commons.lang3.StringUtils;
  */
 public final class ToscaPolicyTranslatorUtils {
     private static final ObjectFactory factory = new ObjectFactory();
+    private static final StandardCoder CODER = new StandardCoder();
 
     private ToscaPolicyTranslatorUtils() {
         super();
@@ -240,4 +246,34 @@ public final class ToscaPolicyTranslatorUtils {
         newCondition.setExpression(factory.createApply(applyFunction));
         return newCondition;
     }
+
+    /**
+     * Decodes TOSCA Policy properties into a particular type and validates the result.
+     *
+     * @param <T> desired type
+     * @param properties properties to be decoded
+     * @param clazz desired class
+     * @return the decoded properties
+     * @throws ToscaPolicyConversionException if the properties cannot be decoded or are
+     *         invalid
+     */
+    public static <T> T decodeProperties(Map<String, Object> properties, Class<T> clazz)
+                    throws ToscaPolicyConversionException {
+
+        if (properties == null) {
+            throw new ToscaPolicyConversionException(
+                            "Cannot decode " + clazz.getSimpleName() + " from null properties");
+        }
+
+        try {
+            T data = CODER.convert(properties, clazz);
+            BeanValidationResult result = new BeanValidator().validateTop("properties", data);
+            if (!result.isValid()) {
+                throw new ToscaPolicyConversionException(result.getResult());
+            }
+            return data;
+        } catch (CoderException e) {
+            throw new ToscaPolicyConversionException("Cannot decode " + clazz.getSimpleName() + " from properties", e);
+        }
+    }
 }

diff --git a/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/ToscaPolicyTranslatorUtilsTest.java b/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/ToscaPolicyTranslatorUtilsTest.java
index 99627f6..e684cde 100644 (file)
--- a/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/ToscaPolicyTranslatorUtilsTest.java
+++ b/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/ToscaPolicyTranslatorUtilsTest.java
@@ -23,11 +23,14 @@
 package org.onap.policy.pdp.xacml.application.common;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.Assert.assertTrue;
 
 import com.att.research.xacml.api.XACML3;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Modifier;
+import java.util.Map;
+import lombok.Getter;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.AllOfType;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.AnyOfType;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.ApplyType;
@@ -39,6 +42,8 @@ import oasis.names.tc.xacml._3_0.core.schema.wd_17.ObjectFactory;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.TargetType;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.VariableReferenceType;
 import org.junit.Test;
+import org.onap.policy.common.parameters.annotations.NotNull;
+import org.onap.policy.common.utils.coder.CoderException;
 
 public class ToscaPolicyTranslatorUtilsTest {
     private static final ObjectFactory factory = new ObjectFactory();
@@ -110,4 +115,30 @@ public class ToscaPolicyTranslatorUtilsTest {
         assertThat(((ApplyType) obj).getFunctionId()).isEqualTo(XACML3.ID_FUNCTION_AND.stringValue());
         assertThat(((ApplyType) obj).getExpression()).hasSize(2);
     }
+
+    @Test
+    public void testDecodeProperties() throws ToscaPolicyConversionException {
+        Data data = ToscaPolicyTranslatorUtils.decodeProperties(Map.of("value", 20), Data.class);
+        assertThat(data.getValue()).isEqualTo(20);
+
+        // null value - invalid
+        assertThatThrownBy(() -> ToscaPolicyTranslatorUtils.decodeProperties(Map.of(), Data.class))
+                        .isInstanceOf(ToscaPolicyConversionException.class).hasMessageContaining("item \"value\"");
+
+        // value is not an integer - cannot even decode it
+        assertThatThrownBy(() -> ToscaPolicyTranslatorUtils.decodeProperties(Map.of("value", "abc"), Data.class))
+                        .isInstanceOf(ToscaPolicyConversionException.class).getCause()
+                        .isInstanceOf(CoderException.class);
+
+        // null properties - cannot even decode
+        assertThatThrownBy(() -> ToscaPolicyTranslatorUtils.decodeProperties(null, Data.class))
+                        .isInstanceOf(ToscaPolicyConversionException.class)
+                        .hasMessage("Cannot decode Data from null properties");
+    }
+
+    @Getter
+    @NotNull
+    public static class Data {
+        private Integer value;
+    }
 }

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 0eea972..d606cc2 100644 (file)
--- 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
@@ -32,10 +32,13 @@ import com.att.research.xacml.api.Result;
 import com.att.research.xacml.api.XACML3;
 import com.att.research.xacml.std.IdentifierImpl;
 import com.att.research.xacml.std.annotations.RequestParser;
+import com.google.gson.annotations.SerializedName;
 import java.time.OffsetDateTime;
 import java.time.OffsetTime;
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
+import lombok.Getter;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.AllOfType;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.AnyOfType;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.ApplyType;
@@ -50,7 +53,9 @@ import oasis.names.tc.xacml._3_0.core.schema.wd_17.RuleType;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.TargetType;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.VariableDefinitionType;
 import oasis.names.tc.xacml._3_0.core.schema.wd_17.VariableReferenceType;
-import org.apache.commons.lang3.StringUtils;
+import org.onap.policy.common.parameters.annotations.NotBlank;
+import org.onap.policy.common.parameters.annotations.NotNull;
+import org.onap.policy.common.parameters.annotations.Valid;
 import org.onap.policy.models.decisions.concepts.DecisionRequest;
 import org.onap.policy.models.decisions.concepts.DecisionResponse;
 import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
@@ -171,9 +176,8 @@ public class GuardTranslator implements ToscaPolicyTranslator {
         //
         // Add in our variable definition
         //
-        Object timeRange = toscaPolicy.getProperties().get(FIELD_TIMERANGE);
-        if (timeRange != null) {
-            VariableReferenceType variable = this.createTimeRangeVariable(timeRange, newPolicyType);
+        VariableReferenceType variable = this.createTimeRangeVariable(toscaPolicy.getProperties(), newPolicyType);
+        if (variable != null) {
             //
             // Update all the rules to have conditions for this variable
             //
@@ -295,20 +299,25 @@ public class GuardTranslator implements ToscaPolicyTranslator {
     protected TargetType generateTargetType(Map<String, Object> properties, boolean addTargets)
             throws ToscaPolicyConversionException {
         //
+        // Decode the definition from the policy's properties
+        //
+        TargetTypeDefinition targetTypeDef =
+                        ToscaPolicyTranslatorUtils.decodeProperties(properties, TargetTypeDefinition.class);
+        //
         // Go through potential properties
         //
         var allOf = new AllOfType();
-        if (properties.containsKey(FIELD_ACTOR)) {
-            addMatch(allOf, properties.get(FIELD_ACTOR), ToscaDictionary.ID_RESOURCE_GUARD_ACTOR);
+        if (targetTypeDef.getActor() != null) {
+            addMatch(allOf, targetTypeDef.getActor(), ToscaDictionary.ID_RESOURCE_GUARD_ACTOR);
         }
-        if (properties.containsKey(FIELD_OPERATION)) {
-            addMatch(allOf, properties.get(FIELD_OPERATION), ToscaDictionary.ID_RESOURCE_GUARD_RECIPE);
+        if (targetTypeDef.getOperation() != null) {
+            addMatch(allOf, targetTypeDef.getOperation(), ToscaDictionary.ID_RESOURCE_GUARD_RECIPE);
         }
-        if (addTargets && properties.containsKey(FIELD_TARGET)) {
-            addMatch(allOf, properties.get(FIELD_TARGET), ToscaDictionary.ID_RESOURCE_GUARD_TARGETID);
+        if (addTargets && targetTypeDef.getTarget() != null) {
+            addMatch(allOf, targetTypeDef.getTarget(), ToscaDictionary.ID_RESOURCE_GUARD_TARGETID);
         }
-        if (properties.containsKey(FIELD_CONTROLLOOP)) {
-            addMatch(allOf, properties.get(FIELD_CONTROLLOOP), ToscaDictionary.ID_RESOURCE_GUARD_CLNAME);
+        if (targetTypeDef.getId() != null) {
+            addMatch(allOf, targetTypeDef.getId(), ToscaDictionary.ID_RESOURCE_GUARD_CLNAME);
         }
         //
         // Create target
@@ -358,16 +367,12 @@ public class GuardTranslator implements ToscaPolicyTranslator {
         return allOf;
     }
 
-    @SuppressWarnings("rawtypes")
-    protected void addTimeRangeMatch(AllOfType allOf, Object timeRange)
+    protected void addTimeRangeMatch(AllOfType allOf, TimeRange timeRange)
             throws ToscaPolicyConversionException {
-        if (! (timeRange instanceof Map)) {
-            throw new ToscaPolicyConversionException("timeRange is not a map object " + timeRange.getClass());
-        }
 
         var matchStart = ToscaPolicyTranslatorUtils.buildMatchTypeDesignator(
                 XACML3.ID_FUNCTION_TIME_GREATER_THAN_OR_EQUAL,
-                ((Map) timeRange).get("start_time").toString(),
+                timeRange.getStartTime(),
                 XACML3.ID_DATATYPE_TIME,
                 XACML3.ID_ENVIRONMENT_CURRENT_TIME,
                 XACML3.ID_ATTRIBUTE_CATEGORY_ENVIRONMENT);
@@ -376,7 +381,7 @@ public class GuardTranslator implements ToscaPolicyTranslator {
 
         var matchEnd = ToscaPolicyTranslatorUtils.buildMatchTypeDesignator(
                 XACML3.ID_FUNCTION_TIME_LESS_THAN_OR_EQUAL,
-                ((Map) timeRange).get("end_time").toString(),
+                timeRange.getEndTime(),
                 XACML3.ID_DATATYPE_TIME,
                 XACML3.ID_ENVIRONMENT_CURRENT_TIME,
                 XACML3.ID_ATTRIBUTE_CATEGORY_ENVIRONMENT);
@@ -384,36 +389,22 @@ public class GuardTranslator implements ToscaPolicyTranslator {
         allOf.getMatch().add(matchEnd);
     }
 
-    @SuppressWarnings("rawtypes")
-    protected VariableReferenceType createTimeRangeVariable(Object timeRange, PolicyType newPolicyType)
+    protected VariableReferenceType createTimeRangeVariable(Map<String, Object> properties, PolicyType newPolicyType)
             throws ToscaPolicyConversionException {
         //
-        // Sanity check the properties
+        // Decode the definition from the policy's properties
         //
-        if (! (timeRange instanceof Map)) {
-            throw new ToscaPolicyConversionException("timeRange is not a map object " + timeRange.getClass());
-        }
-        String startTimestamp;
-        String endTimestamp;
-        try {
-            startTimestamp = ((Map) timeRange).get("start_time").toString();
-            endTimestamp = ((Map) timeRange).get("end_time").toString();
-            if (StringUtils.isBlank(startTimestamp)) {
-                throw new ToscaPolicyConversionException("Missing timeRange start_time property");
-            }
-            if (StringUtils.isBlank(endTimestamp)) {
-                throw new ToscaPolicyConversionException("Missing timeRange end_time property");
-            }
-        } catch (ToscaPolicyConversionException e) {
-            throw e;
-        } catch (Exception e) {
-            throw new ToscaPolicyConversionException("Invalid timeRange", e);
+        TimeRangeDefinition timeRangeDef =
+                        ToscaPolicyTranslatorUtils.decodeProperties(properties, TimeRangeDefinition.class);
+        TimeRange timeRange = timeRangeDef.getTimeRange();
+        if (timeRange == null) {
+            return null;
         }
         //
         // Should also be parseable as an ISO8601 timestamp
         //
-        var startTimeObject = parseTimestamp(startTimestamp);
-        var endTimeObject = parseTimestamp(endTimestamp);
+        var startTimeObject = parseTimestamp(timeRange.getStartTime());
+        var endTimeObject = parseTimestamp(timeRange.getEndTime());
         //
         // They should be the same object types. We cannot establish a range
         // between an OffsetDateTime and an OffsetTime
@@ -424,7 +415,8 @@ public class GuardTranslator implements ToscaPolicyTranslator {
         //
         // Create the inner timeInRange ApplyType
         //
-        ApplyType timeInRange = ToscaPolicyTranslatorUtils.generateTimeInRange(startTimestamp, endTimestamp, true);
+        ApplyType timeInRange = ToscaPolicyTranslatorUtils.generateTimeInRange(timeRange.getStartTime(),
+                        timeRange.getEndTime(), true);
         var variable = new VariableDefinitionType();
         variable.setVariableId(VARIABLE_TIMEINRANGE);
         variable.setExpression(new ObjectFactory().createApply(timeInRange));
@@ -462,36 +454,22 @@ public class GuardTranslator implements ToscaPolicyTranslator {
     protected void generateFrequencyRules(ToscaPolicy toscaPolicy, String policyName, PolicyType newPolicyType)
             throws ToscaPolicyConversionException {
         //
-        // We must have the limit
+        // Decode the definition from the policy's properties
         //
-        if (! toscaPolicy.getProperties().containsKey(FIELD_LIMIT)) {
-            throw new ToscaPolicyConversionException("Missing property limit");
-        }
+        FrequencyDefinition frequencyDef = ToscaPolicyTranslatorUtils.decodeProperties(toscaPolicy.getProperties(),
+                        FrequencyDefinition.class);
         //
         // See if its possible to generate a count
         //
-        var limit = ToscaPolicyTranslatorUtils.parseInteger(
-                toscaPolicy.getProperties().get(FIELD_LIMIT).toString());
-        if (limit == null) {
-            throw new ToscaPolicyConversionException("Missing limit value");
-        }
         String timeWindow = null;
-        if (toscaPolicy.getProperties().containsKey(FIELD_TIMEWINDOW)) {
-            var intTimeWindow = ToscaPolicyTranslatorUtils.parseInteger(
-                    toscaPolicy.getProperties().get(FIELD_TIMEWINDOW).toString());
-            if (intTimeWindow == null) {
-                throw new ToscaPolicyConversionException("timeWindow is not an integer");
-            }
-            timeWindow = intTimeWindow.toString();
-        }
-        String timeUnits = null;
-        if (toscaPolicy.getProperties().containsKey(FIELD_TIMEUNITS)) {
-            timeUnits = toscaPolicy.getProperties().get(FIELD_TIMEUNITS).toString();
+        if (frequencyDef.getTimeWindow() != null) {
+            timeWindow = frequencyDef.getTimeWindow().toString();
         }
         //
         // Generate a count
         //
-        final ApplyType countCheck = generateCountCheck(limit, timeWindow, timeUnits);
+        final ApplyType countCheck =
+                        generateCountCheck(frequencyDef.getLimit(), timeWindow, frequencyDef.getTimeUnits());
         //
         // Create our condition
         //
@@ -558,14 +536,16 @@ public class GuardTranslator implements ToscaPolicyTranslator {
     protected void generateMinMaxRules(ToscaPolicy toscaPolicy, String policyName, PolicyType newPolicyType)
             throws ToscaPolicyConversionException {
         //
+        // Decode the definition from the policy's properties
+        //
+        MinMaxDefinition minMaxDef = ToscaPolicyTranslatorUtils.decodeProperties(toscaPolicy.getProperties(),
+                        MinMaxDefinition.class);
+        //
         // Add the target
         //
-        if (! toscaPolicy.getProperties().containsKey(FIELD_TARGET)) {
-            throw new ToscaPolicyConversionException("Missing target field in minmax policy");
-        }
         var matchTarget = ToscaPolicyTranslatorUtils.buildMatchTypeDesignator(
                 XACML3.ID_FUNCTION_STRING_EQUAL,
-                toscaPolicy.getProperties().get(FIELD_TARGET).toString(),
+                minMaxDef.getTarget(),
                 XACML3.ID_DATATYPE_STRING,
                 ToscaDictionary.ID_RESOURCE_GUARD_TARGETID,
                 XACML3.ID_ATTRIBUTE_CATEGORY_RESOURCE);
@@ -573,12 +553,10 @@ public class GuardTranslator implements ToscaPolicyTranslator {
         // For the min, if the # of instances is less than the minimum
         // then allow the scale.
         //
-        Integer min = null;
-        if (toscaPolicy.getProperties().containsKey(FIELD_MIN)) {
-            min = ToscaPolicyTranslatorUtils.parseInteger(toscaPolicy.getProperties().get(FIELD_MIN).toString());
+        if (minMaxDef.getMin() != null) {
             var matchMin = ToscaPolicyTranslatorUtils.buildMatchTypeDesignator(
                     XACML3.ID_FUNCTION_INTEGER_GREATER_THAN,
-                    min.toString(),
+                    minMaxDef.getMin().toString(),
                     XACML3.ID_DATATYPE_INTEGER,
                     ToscaDictionary.ID_RESOURCE_GUARD_VFCOUNT,
                     XACML3.ID_ATTRIBUTE_CATEGORY_RESOURCE);
@@ -586,12 +564,10 @@ public class GuardTranslator implements ToscaPolicyTranslator {
             newPolicyType.getCombinerParametersOrRuleCombinerParametersOrVariableDefinition().add(
                     generateMinMaxRule(matchTarget, matchMin, policyName + ":minrule", "check minimum"));
         }
-        Integer max = null;
-        if (toscaPolicy.getProperties().containsKey(FIELD_MAX)) {
-            max = ToscaPolicyTranslatorUtils.parseInteger(toscaPolicy.getProperties().get(FIELD_MAX).toString());
+        if (minMaxDef.getMax() != null) {
             var matchMax = ToscaPolicyTranslatorUtils.buildMatchTypeDesignator(
                     XACML3.ID_FUNCTION_INTEGER_GREATER_THAN,
-                    max.toString(),
+                    minMaxDef.getMax().toString(),
                     XACML3.ID_DATATYPE_INTEGER,
                     ToscaDictionary.ID_RESOURCE_GUARD_VFCOUNT,
                     XACML3.ID_ATTRIBUTE_CATEGORY_RESOURCE);
@@ -602,7 +578,7 @@ public class GuardTranslator implements ToscaPolicyTranslator {
         //
         // Do we have at least a min or max?
         //
-        if (min == null && max == null) {
+        if (minMaxDef.getMin() == null && minMaxDef.getMax() == null) {
             throw new ToscaPolicyConversionException("Missing min or max field in minmax policy");
         }
     }
@@ -626,25 +602,17 @@ public class GuardTranslator implements ToscaPolicyTranslator {
     protected void generateBlacklistRules(ToscaPolicy toscaPolicy, String policyName, PolicyType newPolicyType)
             throws ToscaPolicyConversionException {
         //
-        // Validate the blacklist exists
-        //
-        if (! toscaPolicy.getProperties().containsKey(FIELD_BLACKLIST)) {
-            throw new ToscaPolicyConversionException("Missing blacklist field");
-        }
+        // Decode the definition from the policy's properties
         //
-        // Get the blacklist, which should be an array or collection.
-        //
-        Object arrayBlacklisted = toscaPolicy.getProperties().get(FIELD_BLACKLIST);
-        if (!(arrayBlacklisted instanceof Collection)) {
-            throw new ToscaPolicyConversionException("Blacklist is not a collection");
-        }
+        BlacklistDefinition blacklistDef = ToscaPolicyTranslatorUtils.decodeProperties(toscaPolicy.getProperties(),
+                        BlacklistDefinition.class);
         //
         // Iterate the entries and create individual AnyOf so each entry is
         // treated as an OR.
         //
         var target = new TargetType();
         var anyOf = new AnyOfType();
-        for (Object blacklisted : ((Collection<?>) arrayBlacklisted)) {
+        for (Object blacklisted : blacklistDef.blacklist) {
             var allOf = new AllOfType();
             this.addMatch(allOf, blacklisted, ToscaDictionary.ID_RESOURCE_GUARD_TARGETID);
             anyOf.getAllOf().add(allOf);
@@ -664,57 +632,42 @@ public class GuardTranslator implements ToscaPolicyTranslator {
         newPolicyType.getCombinerParametersOrRuleCombinerParametersOrVariableDefinition().add(blacklistRule);
     }
 
-    @SuppressWarnings("unchecked")
     protected void generateFilterRules(ToscaPolicy toscaPolicy, String policyName, PolicyType newPolicyType)
             throws ToscaPolicyConversionException {
         //
-        // Validate the algorithm
-        //
-        if (! toscaPolicy.getProperties().containsKey(FIELD_FILTER_ALGORITHM)) {
-            throw new ToscaPolicyConversionException("Missing algorithm");
-        }
-        Object algorithm = toscaPolicy.getProperties().get(FIELD_FILTER_ALGORITHM);
-        if ("whitelist-overrides".equals(algorithm.toString())) {
-            newPolicyType.setRuleCombiningAlgId(XACML3.ID_RULE_PERMIT_OVERRIDES.stringValue());
-        } else if ("blacklist-overrides".equals(algorithm.toString())) {
-            newPolicyType.setRuleCombiningAlgId(XACML3.ID_RULE_DENY_OVERRIDES.stringValue());
-        } else {
-            throw new ToscaPolicyConversionException(
-                    "Unexpected value for algorithm, should be whitelist-overrides or blacklist-overrides");
-        }
+        // Decode the definition from the policy's properties
         //
-        // Validate the filters exist and have the right properties
+        FilterDefinition filterDef = ToscaPolicyTranslatorUtils.decodeProperties(toscaPolicy.getProperties(),
+                        FilterDefinition.class);
         //
-        if (! toscaPolicy.getProperties().containsKey(FIELD_FILTER_FILTERS)) {
-            throw new ToscaPolicyConversionException("Missing filters");
-        }
+        // Set the combining algorithm
         //
-        // Get the filters, which should be an array or collection.
-        //
-        Object arrayFilters = toscaPolicy.getProperties().get(FIELD_FILTER_FILTERS);
-        if (!(arrayFilters instanceof Collection)) {
-            throw new ToscaPolicyConversionException("Filters is not a collection");
+        switch (filterDef.getAlgorithm()) {
+            case "whitelist-overrides":
+                newPolicyType.setRuleCombiningAlgId(XACML3.ID_RULE_PERMIT_OVERRIDES.stringValue());
+                break;
+            case "blacklist-overrides":
+                newPolicyType.setRuleCombiningAlgId(XACML3.ID_RULE_DENY_OVERRIDES.stringValue());
+                break;
+            default:
+                throw new ToscaPolicyConversionException(
+                                "Unexpected value for algorithm, should be whitelist-overrides or blacklist-overrides");
         }
         //
         // Iterate the filters
         //
         var ruleId = 1;
-        for (Object filterAttributes : ((Collection<?>) arrayFilters)) {
-            if (!(filterAttributes instanceof Map)) {
-                throw new ToscaPolicyConversionException("Filter should be a map");
-            }
+        for (FilterAttribute filterAttributes : filterDef.filters) {
             //
-            // All fields must be there
+            // Check fields requiring extra validation
             //
-            String field = validateFilterPropertyField((Map<String, Object>) filterAttributes);
-            String filter = validateFilterPropertyFilter((Map<String, Object>) filterAttributes);
-            Identifier function = validateFilterPropertyFunction((Map<String, Object>) filterAttributes);
-            boolean isBlacklisted = validateFilterPropertyBlacklist((Map<String, Object>) filterAttributes);
+            String field = validateFilterPropertyField(filterAttributes.getField());
+            Identifier function = validateFilterPropertyFunction(filterAttributes.getFunction());
             //
             // Create our filter rule
             //
-            RuleType filterRule = createFilterRule(policyName + ":rule" + ruleId++, field, filter,
-                    function, isBlacklisted);
+            RuleType filterRule = createFilterRule(policyName + ":rule" + ruleId++, field, filterAttributes.getFilter(),
+                            function, filterAttributes.getBlacklist());
             //
             // Add the rule to the policy
             //
@@ -722,79 +675,48 @@ public class GuardTranslator implements ToscaPolicyTranslator {
         }
     }
 
-    private String validateFilterPropertyField(Map<String, Object> filterAttributes)
+    private String validateFilterPropertyField(String field)
             throws ToscaPolicyConversionException {
-        Object field = filterAttributes.get(FIELD_FILTER_FIELD);
-        if (field != null) {
-            switch (field.toString().toLowerCase()) {
-                case "generic-vnf.vnf-name":
-                case "generic-vnf.vnf-id":
-                case "generic-vnf.vnf-type":
-                case "generic-vnf.nf-naming-code":
-                case "vserver.vserver-id":
-                case "cloud-region.cloud-region-id":
-                    return field.toString();
-                default:
-                    throw new ToscaPolicyConversionException("Unexpected value for field in filter");
-            }
+        String fieldLowerCase = field.toLowerCase();
+        switch (fieldLowerCase) {
+            case "generic-vnf.vnf-name":
+            case "generic-vnf.vnf-id":
+            case "generic-vnf.vnf-type":
+            case "generic-vnf.nf-naming-code":
+            case "vserver.vserver-id":
+            case "cloud-region.cloud-region-id":
+                return fieldLowerCase;
+            default:
+                throw new ToscaPolicyConversionException("Unexpected value for field in filter");
         }
-        throw new ToscaPolicyConversionException("Missing \'field\' from filter");
     }
 
-    private String validateFilterPropertyFilter(Map<String, Object> filterAttributes)
+    private Identifier validateFilterPropertyFunction(String function)
             throws ToscaPolicyConversionException {
-        Object filter = filterAttributes.get(FIELD_FILTER_FILTER);
-        if (filter != null) {
-            return filter.toString();
+        switch (function.toLowerCase()) {
+            case "string-equal":
+                return XACML3.ID_FUNCTION_STRING_EQUAL;
+            case "string-equal-ignore-case":
+                return XACML3.ID_FUNCTION_STRING_EQUAL_IGNORE_CASE;
+            case "string-regexp-match":
+                return XACML3.ID_FUNCTION_STRING_REGEXP_MATCH;
+            case "string-contains":
+                return XACML3.ID_FUNCTION_STRING_CONTAINS;
+            case "string-greater-than":
+                return XACML3.ID_FUNCTION_STRING_GREATER_THAN;
+            case "string-greater-than-or-equal":
+                return XACML3.ID_FUNCTION_STRING_GREATER_THAN_OR_EQUAL;
+            case "string-less-than":
+                return XACML3.ID_FUNCTION_STRING_LESS_THAN;
+            case "string-less-than-or-equal":
+                return XACML3.ID_FUNCTION_STRING_LESS_THAN_OR_EQUAL;
+            case "string-starts-with":
+                return XACML3.ID_FUNCTION_STRING_STARTS_WITH;
+            case "string-ends-with":
+                return XACML3.ID_FUNCTION_STRING_ENDS_WITH;
+            default:
+                throw new ToscaPolicyConversionException("Unexpected value for function in filter");
         }
-        throw new ToscaPolicyConversionException("Missing \'filter\' from filter");
-    }
-
-    private Identifier validateFilterPropertyFunction(Map<String, Object> filterAttributes)
-            throws ToscaPolicyConversionException {
-        Object function = filterAttributes.get(FIELD_FILTER_FUNCTION);
-        if (function != null) {
-            switch (function.toString().toLowerCase()) {
-                case "string-equal":
-                    return XACML3.ID_FUNCTION_STRING_EQUAL;
-                case "string-equal-ignore-case":
-                    return XACML3.ID_FUNCTION_STRING_EQUAL_IGNORE_CASE;
-                case "string-regexp-match":
-                    return XACML3.ID_FUNCTION_STRING_REGEXP_MATCH;
-                case "string-contains":
-                    return XACML3.ID_FUNCTION_STRING_CONTAINS;
-                case "string-greater-than":
-                    return XACML3.ID_FUNCTION_STRING_GREATER_THAN;
-                case "string-greater-than-or-equal":
-                    return XACML3.ID_FUNCTION_STRING_GREATER_THAN_OR_EQUAL;
-                case "string-less-than":
-                    return XACML3.ID_FUNCTION_STRING_LESS_THAN;
-                case "string-less-than-or-equal":
-                    return XACML3.ID_FUNCTION_STRING_LESS_THAN_OR_EQUAL;
-                case "string-starts-with":
-                    return XACML3.ID_FUNCTION_STRING_STARTS_WITH;
-                case "string-ends-with":
-                    return XACML3.ID_FUNCTION_STRING_ENDS_WITH;
-                default:
-                    throw new ToscaPolicyConversionException("Unexpected value for function in filter");
-            }
-        }
-        throw new ToscaPolicyConversionException("Missing \'function\' from filter");
-    }
-
-    private boolean validateFilterPropertyBlacklist(Map<String, Object> filterAttributes)
-            throws ToscaPolicyConversionException {
-        Object filter = filterAttributes.get(FIELD_FILTER_BLACKLIST);
-        if (filter != null) {
-            if ("true".equalsIgnoreCase(filter.toString())) {
-                return true;
-            }
-            if ("false".equalsIgnoreCase(filter.toString())) {
-                return false;
-            }
-            throw new ToscaPolicyConversionException("Unexpected value for blacklist in filter");
-        }
-        throw new ToscaPolicyConversionException("Missing \'blacklist\' from filter");
     }
 
     private RuleType createFilterRule(String ruleId, String field, String filter, Identifier function,
@@ -829,4 +751,65 @@ public class GuardTranslator implements ToscaPolicyTranslator {
         return rule;
     }
 
+    @Getter
+    public static class TimeRangeDefinition {
+        private @Valid TimeRange timeRange;
+    }
+
+    @Getter
+    public static class TargetTypeDefinition {
+        private String actor;
+        private String operation;
+        private String target;
+        private String id;
+    }
+
+    @Getter
+    @NotNull
+    @NotBlank
+    public static class TimeRange {
+        @SerializedName("start_time")
+        private String startTime;
+
+        @SerializedName("end_time")
+        private String endTime;
+    }
+
+    @Getter
+    public static class FrequencyDefinition {
+        @NotNull
+        private Integer limit;
+        private Integer timeWindow;
+        private String timeUnits;
+    }
+
+    @Getter
+    public static class MinMaxDefinition {
+        @NotNull
+        private String target;
+        private Integer min;
+        private Integer max;
+    }
+
+    @Getter
+    @NotNull
+    public static class BlacklistDefinition {
+        private List<@NotNull Object> blacklist;
+    }
+
+    @Getter
+    @NotNull
+    public static class FilterDefinition {
+        private String algorithm;
+        private List<@NotNull @Valid FilterAttribute> filters;
+    }
+
+    @Getter
+    @NotNull
+    public static class FilterAttribute {
+        private String field;
+        private String filter;
+        private String function;
+        private Boolean blacklist;
+    }
 }

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 cf8c015..fcd5ac2 100644 (file)
--- 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
@@ -117,27 +117,26 @@ public class GuardTranslatorTest {
         // Expected message for given policy name
         //
         final Map<String, String> name2message = new HashMap<>();
-        name2message.put("frequency-missing-properties", "Missing property limit");
-        name2message.put("frequency-timewindow", "timeWindow is not an integer");
-        name2message.put("frequency-badtimerange_start", "Invalid timeRange");
-        name2message.put("frequency-badtimerange_end", "Invalid timeRange");
+        name2message.put("frequency-missing-properties", "item \"limit\"");
+        name2message.put("frequency-timewindow", "Cannot decode FrequencyDefinition");
+        name2message.put("frequency-badtimerange_start", "item \"startTime\"");
+        name2message.put("frequency-badtimerange_end", "item \"endTime\"");
         name2message.put("frequency-badtimerange_value", "timestamp 99:99:99 could not be parsed");
-        name2message.put("minmax-notarget", "Missing target field in minmax policy");
+        name2message.put("minmax-notarget", "item \"target\"");
         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("blacklist-noblacklist", "item \"blacklist\"");
+        name2message.put("filter-noalgorithm", "item \"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-nofilter", "item \"filters\"");
+        name2message.put("filter-nocollection", "Cannot decode FilterDefinition");
+        name2message.put("filter-noarray", "Cannot decode FilterDefinition");
+        name2message.put("filter-missingfield", "item \"field\"");
         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-missingfilter", "item \"filter\"");
+        name2message.put("filter-missingfunction", "item \"function\"");
         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");
+        name2message.put("filter-missingblacklist", "item \"blacklist\"");
         //
         // Get the policies
         //
@@ -149,7 +148,7 @@ public class GuardTranslatorTest {
 
                 assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() ->
                     translator.convertPolicy(policy)
-                ).withMessageContaining(expectedMsg);
+                ).as(policy.getName()).withMessageContaining(expectedMsg);
             }
         }
     }

diff --git a/applications/guard/src/test/resources/test-bad-policies.yaml b/applications/guard/src/test/resources/test-bad-policies.yaml
index bdc8ba7..d7780c1 100644 (file)
--- a/applications/guard/src/test/resources/test-bad-policies.yaml
+++ b/applications/guard/src/test/resources/test-bad-policies.yaml
@@ -79,12 +79,13 @@ topology_template:
          type_version: 1.0.0
          version: 1.0.0
          properties:
-            badProperty: badValue
+            filters: []
    -  filter-badalgorithm:
          type: onap.policies.controlloop.guard.common.Filter
          type_version: 1.0.0
          version: 1.0.0
          properties:
+            filters: []
             algorithm: idontknow
    -  filter-nofilter:
          type: onap.policies.controlloop.guard.common.Filter
@@ -123,6 +124,9 @@ topology_template:
             algorithm: blacklist-overrides
             filters:
             -  field: notinaai
+               filter: vfwl*
+               function: string-contains
+               blacklist: true
    -  filter-missingfilter:
          type: onap.policies.controlloop.guard.common.Filter
          type_version: 1.0.0
@@ -150,6 +154,7 @@ topology_template:
             -  field: generic-vnf.vnf-name
                filter: vfwl*
                function: notafunction
+               blacklist: true
    -  filter-missingblacklist:
          type: onap.policies.controlloop.guard.common.Filter
          type_version: 1.0.0
@@ -160,14 +165,3 @@ topology_template:
             -  field: generic-vnf.vnf-name
                filter: vfwl*
                function: string-equal
-   -  filter-badblacklist:
-         type: onap.policies.controlloop.guard.common.Filter
-         type_version: 1.0.0
-         version: 1.0.0
-         properties:
-            algorithm: blacklist-overrides
-            filters:
-            -  field: generic-vnf.vnf-name
-               filter: vfwl*
-               function: string-equal
-               blacklist: shouldbeboolean
\ No newline at end of file

diff --git a/applications/native/src/main/java/org/onap/policy/xacml/pdp/application/nativ/NativePdpApplicationTranslator.java b/applications/native/src/main/java/org/onap/policy/xacml/pdp/application/nativ/NativePdpApplicationTranslator.java
index 7302b67..3411d36 100644 (file)
--- a/applications/native/src/main/java/org/onap/policy/xacml/pdp/application/nativ/NativePdpApplicationTranslator.java
+++ b/applications/native/src/main/java/org/onap/policy/xacml/pdp/application/nativ/NativePdpApplicationTranslator.java
@@ -30,13 +30,15 @@ import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
-import java.util.Map;
-import org.apache.commons.collections4.MapUtils;
+import lombok.Getter;
+import org.onap.policy.common.parameters.annotations.NotBlank;
+import org.onap.policy.common.parameters.annotations.NotNull;
 import org.onap.policy.models.decisions.concepts.DecisionRequest;
 import org.onap.policy.models.decisions.concepts.DecisionResponse;
 import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
 import org.onap.policy.pdp.xacml.application.common.ToscaPolicyConversionException;
 import org.onap.policy.pdp.xacml.application.common.ToscaPolicyTranslator;
+import org.onap.policy.pdp.xacml.application.common.ToscaPolicyTranslatorUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -49,7 +51,6 @@ import org.slf4j.LoggerFactory;
 public class NativePdpApplicationTranslator implements ToscaPolicyTranslator {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(NativePdpApplicationTranslator.class);
-    private static final String POLICY = "policy";
 
     public NativePdpApplicationTranslator() {
         super();
@@ -87,14 +88,11 @@ public class NativePdpApplicationTranslator implements ToscaPolicyTranslator {
 
     private String getNativeXacmlPolicy(ToscaPolicy toscaPolicy) throws ToscaPolicyConversionException {
 
-        Map<String, Object> propertyMap = toscaPolicy.getProperties();
-        if (MapUtils.isEmpty(propertyMap) || !propertyMap.containsKey(POLICY)) {
-            throw new ToscaPolicyConversionException("no xacml native policy found in the tosca policy");
-        }
+        NativeDefinition nativeDefinition = ToscaPolicyTranslatorUtils.decodeProperties(toscaPolicy.getProperties(),
+                        NativeDefinition.class);
 
-        var nativePolicyString = propertyMap.get(POLICY).toString();
-        LOGGER.debug("Base64 encoded native xacml policy {}", nativePolicyString);
-        return nativePolicyString;
+        LOGGER.debug("Base64 encoded native xacml policy {}", nativeDefinition.getPolicy());
+        return nativeDefinition.getPolicy();
     }
 
     @Override
@@ -109,4 +107,11 @@ public class NativePdpApplicationTranslator implements ToscaPolicyTranslator {
         //
         return null;
     }
+
+    @Getter
+    public static class NativeDefinition {
+        @NotNull
+        @NotBlank
+        private String policy;
+    }
 }

diff --git a/applications/native/src/test/java/org/onap/policy/xacml/pdp/application/nativ/NativePdpApplicationTest.java b/applications/native/src/test/java/org/onap/policy/xacml/pdp/application/nativ/NativePdpApplicationTest.java
index 3a405a8..83b65e5 100644 (file)
--- a/applications/native/src/test/java/org/onap/policy/xacml/pdp/application/nativ/NativePdpApplicationTest.java
+++ b/applications/native/src/test/java/org/onap/policy/xacml/pdp/application/nativ/NativePdpApplicationTest.java
@@ -160,15 +160,15 @@ public class NativePdpApplicationTest {
                 if ("bad.base64".equals(policy.getName())) {
                     assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() ->
                         translator.convertPolicy(policy)
-                    ).withMessageContaining("error on Base64 decoding the native policy");
+                    ).as(policy.getName()).withMessageContaining("error on Base64 decoding the native policy");
                 } else if ("bad.noproperties".equals(policy.getName())) {
                     assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() ->
                         translator.convertPolicy(policy)
-                    ).withMessageContaining("no xacml native policy found in the tosca policy");
+                    ).as(policy.getName()).withMessageContaining("Cannot decode NativeDefinition from null properties");
                 } else if ("bad.policy".equals(policy.getName())) {
                     assertThatExceptionOfType(ToscaPolicyConversionException.class).isThrownBy(() ->
                         translator.convertPolicy(policy)
-                    ).withMessageContaining("Invalid XACML Policy");
+                    ).as(policy.getName()).withMessageContaining("Invalid XACML Policy");
                 }
             }
         }

diff --git a/applications/optimization/src/main/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTranslator.java b/applications/optimization/src/main/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTranslator.java
index 068245f..e1fe2dc 100644 (file)
--- a/applications/optimization/src/main/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTranslator.java
+++ b/applications/optimization/src/main/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTranslator.java
@@ -147,22 +147,13 @@ public class OptimizationPdpApplicationTranslator extends StdMatchableTranslator
 
     }
 
-    @SuppressWarnings("unchecked")
     private static PolicyType addSubscriberNameIntoTarget(PolicyType policy,
             Map<String, Object> subscriberProperties) throws ToscaPolicyConversionException {
         //
-        // Find the subscriber names
-        //
-        Object subscriberNames = subscriberProperties.get("subscriberName");
-        if (subscriberNames == null) {
-            throw new ToscaPolicyConversionException("Missing subscriberName property");
-        }
-        //
         // Iterate through all the subscriber names
         //
         var anyOf = new AnyOfType();
-        for (Object subscriberName : subscriberNames instanceof Collection ? (List<Object>) subscriberNames :
-            Arrays.asList(subscriberNames)) {
+        for (Object subscriberName : getPropAsList(subscriberProperties, "subscriberName")) {
 
             var match = ToscaPolicyTranslatorUtils.buildMatchTypeDesignator(
                     XACML3.ID_FUNCTION_STRING_EQUAL,
@@ -183,17 +174,9 @@ public class OptimizationPdpApplicationTranslator extends StdMatchableTranslator
         return policy;
     }
 
-    @SuppressWarnings("unchecked")
     private static AdviceExpressionsType generateSubscriberAdvice(Map<String, Object> subscriberProperties)
             throws ToscaPolicyConversionException {
         //
-        // Get the subscriber role
-        //
-        Object role = subscriberProperties.get(FIELD_SUBSCRIBER_ROLE);
-        if (role == null || StringUtils.isBlank(role.toString())) {
-            throw new ToscaPolicyConversionException("Missing subscriberRole");
-        }
-        //
         // Create our subscriber advice expression
         //
         var adviceExpression = new AdviceExpressionType();
@@ -205,18 +188,14 @@ public class OptimizationPdpApplicationTranslator extends StdMatchableTranslator
         generateSubscriberAdviceAttributes(
                 adviceExpression,
                 ToscaDictionary.ID_ADVICE_OPTIMIZATION_SUBSCRIBER_ROLE,
-                role instanceof Collection ? (List<Object>) role : Arrays.asList(role));
+                getPropAsList(subscriberProperties, FIELD_SUBSCRIBER_ROLE));
         //
         // Get the provision status
         //
-        Object provision = subscriberProperties.get(FIELD_PROV_STATUS);
-        if (provision == null || StringUtils.isBlank(provision.toString())) {
-            throw new ToscaPolicyConversionException("Missing provStatus");
-        }
-        adviceExpression = generateSubscriberAdviceAttributes(
+        generateSubscriberAdviceAttributes(
                 adviceExpression,
                 ToscaDictionary.ID_ADVICE_OPTIMIZATION_SUBSCRIBER_STATUS,
-                role instanceof Collection ? (List<Object>) provision : Arrays.asList(role));
+                getPropAsList(subscriberProperties, FIELD_PROV_STATUS));
         //
         // Add it to the overall expressions
         //
@@ -228,7 +207,7 @@ public class OptimizationPdpApplicationTranslator extends StdMatchableTranslator
         return adviceExpressions;
     }
 
-    private static AdviceExpressionType generateSubscriberAdviceAttributes(AdviceExpressionType adviceExpression,
+    private static void generateSubscriberAdviceAttributes(AdviceExpressionType adviceExpression,
             Identifier attributeId, Collection<Object> adviceAttribute) {
         for (Object attribute : adviceAttribute) {
             var value = new AttributeValueType();
@@ -242,9 +221,17 @@ public class OptimizationPdpApplicationTranslator extends StdMatchableTranslator
 
             adviceExpression.getAttributeAssignmentExpression().add(assignment);
         }
-        //
-        // Return for convenience
-        //
-        return adviceExpression;
+    }
+
+    @SuppressWarnings("unchecked")
+    private static List<Object> getPropAsList(Map<String, Object> properties, String fieldName)
+                    throws ToscaPolicyConversionException {
+
+        Object raw = properties.get(fieldName);
+        if (raw == null || StringUtils.isBlank(raw.toString())) {
+            throw new ToscaPolicyConversionException("Missing " + fieldName);
+        }
+
+        return raw instanceof Collection ? (List<Object>) raw : Arrays.asList(raw);
     }
 }