From 8d54e1d7e53651432932e286f8831f0b4db1f22a Mon Sep 17 00:00:00 2001 From: Pamela Dragosh Date: Fri, 13 Sep 2019 12:12:45 -0400 Subject: 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 --- .../std/StdCombinedPolicyResultsTranslator.java | 1 - .../common/std/StdMatchableTranslator.java | 10 +- .../xacml/application/common/std/StdOnapPip.java | 4 +- .../application/guard/LegacyGuardTranslator.java | 132 +++++++++++---------- .../main/parameters/XacmlPdpParameterHandler.java | 2 +- .../pdpx/main/rest/provider/DecisionProvider.java | 8 +- pom.xml | 3 +- 7 files changed, 74 insertions(+), 86 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 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()); // diff --git a/main/src/main/java/org/onap/policy/pdpx/main/parameters/XacmlPdpParameterHandler.java b/main/src/main/java/org/onap/policy/pdpx/main/parameters/XacmlPdpParameterHandler.java index e8ddd1dc..9aaf3dae 100644 --- a/main/src/main/java/org/onap/policy/pdpx/main/parameters/XacmlPdpParameterHandler.java +++ b/main/src/main/java/org/onap/policy/pdpx/main/parameters/XacmlPdpParameterHandler.java @@ -57,7 +57,7 @@ public class XacmlPdpParameterHandler { } catch (final Exception e) { final String errorMessage = "error reading parameters from \"" + arguments.getConfigurationFilePath() + "\"\n" + "(" + e.getClass().getSimpleName() + "):" + e.getMessage(); - LOGGER.error(errorMessage, e); + LOGGER.error(errorMessage); throw new PolicyXacmlPdpException(errorMessage, e); } diff --git a/main/src/main/java/org/onap/policy/pdpx/main/rest/provider/DecisionProvider.java b/main/src/main/java/org/onap/policy/pdpx/main/rest/provider/DecisionProvider.java index 67e696aa..ec687957 100644 --- a/main/src/main/java/org/onap/policy/pdpx/main/rest/provider/DecisionProvider.java +++ b/main/src/main/java/org/onap/policy/pdpx/main/rest/provider/DecisionProvider.java @@ -52,13 +52,7 @@ public class DecisionProvider { // // Found application for action // - Pair decision; - try { - decision = application.makeDecision(request); - } catch (Exception e) { - LOGGER.error("makeDecision failed", e); - throw e; - } + Pair decision = application.makeDecision(request); // // Calculate statistics // diff --git a/pom.xml b/pom.xml index c52799a7..608feff3 100644 --- a/pom.xml +++ b/pom.xml @@ -44,9 +44,8 @@ ${project.basedir}/../target/code-coverage/jacoco-ut.exec ${project.basedir}/../target/code-coverage/jacoco-it.exec reuseReports - 1.6.0-SNAPSHOT - 2.1.3 + 2.2.0-SNAPSHOT -- cgit 1.2.3-korg