Upgrade to Tosca derivedFrom fix

Upgrade to models fix for derivedFrom() append of 0.0.0
And some sonar fixes for:

Exceptions should be either logged or rethrown but not both
Preconditions" and logging arguments should not require evaluation
Reduced cognitive complexity

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

4 files changed, 71 insertions, 76 deletions

diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdCombinedPolicyResultsTranslator.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdCombinedPolicyResultsTranslator.java
index bd10fb2e..12337d20 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdCombinedPolicyResultsTranslator.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdCombinedPolicyResultsTranslator.java
@@ -112,7 +112,6 @@ public class StdCombinedPolicyResultsTranslator implements ToscaPolicyTranslator
         try {
             jsonPolicy = coder.encode(toscaPolicy);
         } catch (CoderException e) {
-            LOGGER.error("Failed to encode policy to json", e);
             throw new ToscaPolicyConversionException(e);
         }
         addObligation(rule, jsonPolicy);
diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java
index 0575ef1b..7142d43d 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java
@@ -469,14 +469,8 @@ public class StdMatchableTranslator implements ToscaPolicyTranslator {
             //
             // Or do we assume 1.0.0?
             //
-            String strDerivedFrom = childPolicyType.getDerivedFrom();
-            //
-            // Hack that fixes policy/models appending 0.0.0 to the derivedFrom name
-            //
-            if (strDerivedFrom.endsWith("0.0.0")) {
-                strDerivedFrom = strDerivedFrom.substring(0, strDerivedFrom.length() - "0.0.0".length() - 1);
-            }
-            ToscaPolicyTypeIdentifier parentId = new ToscaPolicyTypeIdentifier(strDerivedFrom, "1.0.0");
+            ToscaPolicyTypeIdentifier parentId = new ToscaPolicyTypeIdentifier(childPolicyType.getDerivedFrom(),
+                    "1.0.0");
             //
             // Find the policy type
             //
diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdOnapPip.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdOnapPip.java
index c511a9ad..69838599 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdOnapPip.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdOnapPip.java
@@ -105,12 +105,12 @@ public abstract class StdOnapPip extends StdConfigurableEngine {
         try {
             pipResponse = pipFinder.getMatchingAttributes(pipRequest, this);
             if (pipResponse.getStatus() != null && !pipResponse.getStatus().isOk()) {
-                logger.info("get attribute error retrieving {}: {}", pipRequest.getAttributeId().stringValue(),
+                logger.info("get attribute error retrieving {}: {}", pipRequest.getAttributeId(),
                                 pipResponse.getStatus());
                 pipResponse = null;
             }
             if (pipResponse != null && pipResponse.getAttributes().isEmpty()) {
-                logger.info("No value for {}", pipRequest.getAttributeId().stringValue());
+                logger.info("No value for {}", pipRequest.getAttributeId());
                 pipResponse = null;
             }
         } catch (PIPException ex) {
diff --git a/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/LegacyGuardTranslator.java b/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/LegacyGuardTranslator.java
index 932db959..3ccbd9cd 100644
--- a/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/LegacyGuardTranslator.java
+++ b/applications/guard/src/main/java/org/onap/policy/xacml/pdp/application/guard/LegacyGuardTranslator.java
@@ -68,6 +68,8 @@ public class LegacyGuardTranslator implements ToscaPolicyTranslator {
     private static final String FIELD_GUARD_ACTIVE_START = "guardActiveStart";
     private static final String FIELD_GUARD_ACTIVE_END = "guardActiveEnd";
     private static final String FIELD_TARGET = "targets";
+    private static final String DESC_DEFAULT = "Default is to PERMIT if the policy matches.";
+    private static final String ID_RULE = ":rule";
 
     public LegacyGuardTranslator() {
         super();
@@ -193,7 +195,7 @@ public class LegacyGuardTranslator implements ToscaPolicyTranslator {
             //
             // Add in the Policy Version
             //
-            policy.setVersion(map.get("policy-version").toString());
+            policy.setVersion(map.get("policy-version"));
         }
         return policy;
     }
@@ -209,10 +211,8 @@ public class LegacyGuardTranslator implements ToscaPolicyTranslator {
         if (properties.containsKey("recipe")) {
             addMatch(allOf, properties.get("recipe"), ToscaDictionary.ID_RESOURCE_GUARD_RECIPE);
         }
-        if (addTargets) {
-            if (properties.containsKey("targets")) {
-                addMatch(allOf, properties.get("targets"), ToscaDictionary.ID_RESOURCE_GUARD_TARGETID);
-            }
+        if (addTargets && properties.containsKey(FIELD_TARGET)) {
+            addMatch(allOf, properties.get(FIELD_TARGET), ToscaDictionary.ID_RESOURCE_GUARD_TARGETID);
         }
         if (properties.containsKey("clname")) {
             addMatch(allOf, properties.get("clname"), ToscaDictionary.ID_RESOURCE_GUARD_CLNAME);
@@ -333,8 +333,8 @@ public class LegacyGuardTranslator implements ToscaPolicyTranslator {
         // Now we can create our rule
         //
         RuleType permit = new RuleType();
-        permit.setDescription("Default is to PERMIT if the policy matches.");
-        permit.setRuleId(policyName + ":rule");
+        permit.setDescription(DESC_DEFAULT);
+        permit.setRuleId(policyName + ID_RULE);
         permit.setEffect(EffectType.PERMIT);
         permit.setTarget(new TargetType());
         //
@@ -344,7 +344,7 @@ public class LegacyGuardTranslator implements ToscaPolicyTranslator {
         //
         // TODO Add the advice - Is the request id needed to be returned?
         //
-        // permit.setAdviceExpressions(adviceExpressions);
+        // permit . setAdviceExpressions (adviceExpressions)
         //
         // Done
         //
@@ -391,13 +391,25 @@ public class LegacyGuardTranslator implements ToscaPolicyTranslator {
         // Create our rule
         //
         RuleType permit = new RuleType();
-        permit.setDescription("Default is to PERMIT if the policy matches.");
-        permit.setRuleId(policyName + ":rule");
+        permit.setDescription(DESC_DEFAULT);
+        permit.setRuleId(policyName + ID_RULE);
         permit.setEffect(EffectType.PERMIT);
         permit.setTarget(new TargetType());
         //
-        // Create our condition
+        // Create the condition
+        //
+        permit.setCondition(createCondition(timeRange, minApply, maxApply));
+        //
+        // TODO Add the advice - Is the request id needed to be returned?
+        //
+        // permit . setAdviceExpressions (adviceExpressions)
+        //
+        // Done
         //
+        return permit;
+    }
+
+    private static ConditionType createCondition(ApplyType timeRange, ApplyType minApply, ApplyType maxApply) {
         final ConditionType condition = new ConditionType();
         //
         // Check if we have all the fields (this can be a little
@@ -419,63 +431,53 @@ public class LegacyGuardTranslator implements ToscaPolicyTranslator {
             // Add into the condition
             //
             condition.setExpression(factory.createApply(applyAnd));
+
+            return condition;
+        }
+        //
+        // At least one of these applies is null. We need at least
+        // two to require the And apply. Otherwise there is no need
+        // for an outer And apply as the single condition can work
+        // on its own.
+        //
+        if (timeRange != null && minApply == null && maxApply == null) {
+            //
+            // Only the time range check is necessary
+            //
+            condition.setExpression(factory.createApply(timeRange));
+        } else if (timeRange == null && minApply != null && maxApply == null) {
+            //
+            // Only the min check is necessary
+            //
+            condition.setExpression(factory.createApply(minApply));
+        } else if (timeRange == null && minApply == null) {
+            //
+            // Only the max check is necessary
+            //
+            condition.setExpression(factory.createApply(maxApply));
         } else {
             //
-            // At least one of these applies is null. We need at least
-            // two to require the And apply. Otherwise there is no need
-            // for an outer And apply as the single condition can work
-            // on its own.
+            // Ok we will need an outer And and have at least the
+            // time range and either min or max check
             //
-            if (timeRange != null && minApply == null && maxApply == null) {
-                //
-                // Only the time range check is necessary
-                //
-                condition.setExpression(factory.createApply(timeRange));
-            } else if (timeRange == null && minApply != null && maxApply == null) {
-                //
-                // Only the min check is necessary
-                //
-                condition.setExpression(factory.createApply(minApply));
-            } else if (timeRange == null && minApply == null) {
-                //
-                // Only the max check is necessary
-                //
-                condition.setExpression(factory.createApply(maxApply));
-            } else {
-                //
-                // Ok we will need an outer And and have at least the
-                // time range and either min or max check
-                //
-                ApplyType applyAnd = new ApplyType();
-                applyAnd.setDescription("return true if all the apply's are true.");
-                applyAnd.setFunctionId(XACML3.ID_FUNCTION_AND.stringValue());
-                if (timeRange != null) {
-                    applyAnd.getExpression().add(factory.createApply(timeRange));
-                }
-                if (minApply != null) {
-                    applyAnd.getExpression().add(factory.createApply(minApply));
-                }
-                if (maxApply != null) {
-                    applyAnd.getExpression().add(factory.createApply(maxApply));
-                }
-                //
-                // Add into the condition
-                //
-                condition.setExpression(factory.createApply(applyAnd));
+            ApplyType applyAnd = new ApplyType();
+            applyAnd.setDescription("return true if all the apply's are true.");
+            applyAnd.setFunctionId(XACML3.ID_FUNCTION_AND.stringValue());
+            if (timeRange != null) {
+                applyAnd.getExpression().add(factory.createApply(timeRange));
+            }
+            if (minApply != null) {
+                applyAnd.getExpression().add(factory.createApply(minApply));
             }
+            if (maxApply != null) {
+                applyAnd.getExpression().add(factory.createApply(maxApply));
+            }
+            //
+            // Add into the condition
+            //
+            condition.setExpression(factory.createApply(applyAnd));
         }
-        //
-        // Add the condition
-        //
-        permit.setCondition(condition);
-        //
-        // TODO Add the advice - Is the request id needed to be returned?
-        //
-        // permit.setAdviceExpressions(adviceExpressions);
-        //
-        // Done
-        //
-        return permit;
+        return condition;
     }
 
     private static RuleType generateBlacklistPermit(String policyName, Map<String, Object> properties) {
@@ -506,8 +508,8 @@ public class LegacyGuardTranslator implements ToscaPolicyTranslator {
         // Create our rule
         //
         RuleType permit = new RuleType();
-        permit.setDescription("Default is to PERMIT if the policy matches.");
-        permit.setRuleId(policyName + ":rule");
+        permit.setDescription(DESC_DEFAULT);
+        permit.setRuleId(policyName + ID_RULE);
         permit.setEffect(EffectType.PERMIT);
         permit.setTarget(new TargetType());
         //