From 1253af9756e8a7027fade727da68b38dff2d5159 Mon Sep 17 00:00:00 2001 From: Pamela Dragosh Date: Thu, 27 Sep 2018 12:28:19 -0400 Subject: Fix more sonar issues Most of these are minor issues along with a few others. They have been bugging me so I thought I would get them done. Some also do not show up on sonarlint. Issue-ID: POLICY-1129 Change-Id: I31f2765f6babdfa80b23f0589daacc98da8d2002 Signed-off-by: Pamela Dragosh --- .../common/policy-yaml/checkstyle-suppressions.xml | 2 +- .../controlloop/compiler/ControlLoopCompiler.java | 64 +++++--- .../org/onap/policy/controlloop/policy/Policy.java | 174 ++++++++++----------- .../org/onap/policy/controlloop/policy/Target.java | 32 ++-- .../policy/builder/BuilderException.java | 4 +- .../controlloop/policy/guard/Constraint.java | 20 ++- .../controlloop/policy/guard/GuardPolicy.java | 80 +++++----- .../controlloop/policy/guard/MatchParameters.java | 64 ++++---- .../builder/impl/ControlLoopGuardBuilderImpl.java | 2 +- 9 files changed, 237 insertions(+), 205 deletions(-) (limited to 'controlloop/common/policy-yaml') diff --git a/controlloop/common/policy-yaml/checkstyle-suppressions.xml b/controlloop/common/policy-yaml/checkstyle-suppressions.xml index a23b257a6..01e66442b 100644 --- a/controlloop/common/policy-yaml/checkstyle-suppressions.xml +++ b/controlloop/common/policy-yaml/checkstyle-suppressions.xml @@ -25,6 +25,6 @@ diff --git a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/compiler/ControlLoopCompiler.java b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/compiler/ControlLoopCompiler.java index 76c6f8f14..f18752a62 100644 --- a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/compiler/ControlLoopCompiler.java +++ b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/compiler/ControlLoopCompiler.java @@ -268,7 +268,7 @@ public class ControlLoopCompiler implements Serializable { String policyId, String connectedPolicy, FinalResultNodeWrapper finalResultNodeWrapper, PolicyResult policyResult, NodeWrapper node) throws CompilerException { - FinalResult finalResult = FinalResult.toResult(finalResultNodeWrapper.getID()); + FinalResult finalResult = FinalResult.toResult(finalResultNodeWrapper.getId()); if (FinalResult.isResult(connectedPolicy, finalResult)) { graph.addEdge(node, finalResultNodeWrapper, new LabeledEdge(node, finalResultNodeWrapper, new FinalResultEdgeWrapper(finalResult))); @@ -294,7 +294,7 @@ public class ControlLoopCompiler implements Serializable { validatePolicyNodeWrapper(graph, node, callback); } for (LabeledEdge edge : graph.outgoingEdgesOf(node)) { - LOGGER.info(edge.from.getID() + " invokes " + edge.to.getID() + " upon " + edge.edge.getID()); + LOGGER.info("{} invokes {} upon {}", edge.from.getId(), edge.to.getId(), edge.edge.getId()); } } } @@ -302,7 +302,7 @@ public class ControlLoopCompiler implements Serializable { private static void validateTriggerNodeWrapper(DirectedGraph graph, NodeWrapper node) throws CompilerException { if (LOGGER.isDebugEnabled()) { - LOGGER.info("Trigger Node {}", node.toString()); + LOGGER.info("Trigger Node {}", node); } if (graph.inDegreeOf(node) > 0 ) { // @@ -321,7 +321,7 @@ public class ControlLoopCompiler implements Serializable { private static void validateFinalResultNodeWrapper(DirectedGraph graph, NodeWrapper node) throws CompilerException { if (LOGGER.isDebugEnabled()) { - LOGGER.info("FinalResult Node {}", node.toString()); + LOGGER.info("FinalResult Node {}", node); } // // FinalResult nodes should NEVER have an out edge @@ -334,7 +334,7 @@ public class ControlLoopCompiler implements Serializable { private static void validatePolicyNodeWrapper(DirectedGraph graph, NodeWrapper node, ControlLoopCompilerCallback callback) throws CompilerException { if (LOGGER.isDebugEnabled()) { - LOGGER.info("Policy Node {}", node.toString()); + LOGGER.info("Policy Node {}", node); } // // All Policy Nodes should have the 5 out degrees defined. @@ -346,16 +346,24 @@ public class ControlLoopCompiler implements Serializable { // All Policy Nodes should have at least 1 in degrees // if (graph.inDegreeOf(node) == 0 && callback != null) { - callback.onWarning("Policy " + node.getID() + " is not reachable."); + callback.onWarning("Policy " + node.getId() + " is not reachable."); } } private static boolean okToAdd(Policy operPolicy, ControlLoopCompilerCallback callback) { boolean isOk = isPolicyIdOk(operPolicy, callback); - isOk = isActorOk(operPolicy, callback) ? isOk : false; - isOk = isRecipeOk(operPolicy, callback) ? isOk : false; - isOk = isTargetOk(operPolicy, callback) ? isOk : false; - isOk = arePolicyResultsOk(operPolicy, callback) ? isOk : false; + if (! isActorOk(operPolicy, callback)) { + isOk = false; + } + if (! isRecipeOk(operPolicy, callback)) { + isOk = false; + } + if (! isTargetOk(operPolicy, callback) ) { + isOk = false; + } + if (! arePolicyResultsOk(operPolicy, callback) ) { + isOk = false; + } return isOk; } @@ -471,11 +479,21 @@ public class ControlLoopCompiler implements Serializable { // Check that policy results are connected to either default final * or another policy // boolean isOk = isSuccessPolicyResultOk(operPolicy, callback); - isOk = isFailurePolicyResultOk(operPolicy, callback) ? isOk : false; - isOk = isFailureRetriesPolicyResultOk(operPolicy, callback) ? isOk : false; - isOk = isFailureTimeoutPolicyResultOk(operPolicy, callback) ? isOk : false; - isOk = isFailureExceptionPolicyResultOk(operPolicy, callback) ? isOk : false; - isOk = isFailureGuardPolicyResultOk(operPolicy, callback) ? isOk : false; + if (! isFailurePolicyResultOk(operPolicy, callback) ) { + isOk = false; + } + if (! isFailureRetriesPolicyResultOk(operPolicy, callback) ) { + isOk = false; + } + if (! isFailureTimeoutPolicyResultOk(operPolicy, callback) ) { + isOk = false; + } + if (! isFailureExceptionPolicyResultOk(operPolicy, callback) ) { + isOk = false; + } + if (! isFailureGuardPolicyResultOk(operPolicy, callback) ) { + isOk = false; + } return isOk; } @@ -562,7 +580,7 @@ public class ControlLoopCompiler implements Serializable { @FunctionalInterface private interface NodeWrapper extends Serializable { - public String getID(); + public String getId(); } private static class TriggerNodeWrapper implements NodeWrapper { @@ -579,7 +597,7 @@ public class ControlLoopCompiler implements Serializable { } @Override - public String getID() { + public String getId() { return closedLoopControlName; } @@ -599,7 +617,7 @@ public class ControlLoopCompiler implements Serializable { } @Override - public String getID() { + public String getId() { return result.toString(); } } @@ -618,14 +636,14 @@ public class ControlLoopCompiler implements Serializable { } @Override - public String getID() { + public String getId() { return policy.getId(); } } @FunctionalInterface private interface EdgeWrapper extends Serializable { - public String getID(); + public String getId(); } @@ -638,7 +656,7 @@ public class ControlLoopCompiler implements Serializable { } @Override - public String getID() { + public String getId() { return trigger; } @@ -664,7 +682,7 @@ public class ControlLoopCompiler implements Serializable { } @Override - public String getID() { + public String getId() { return policyResult.toString(); } @@ -685,7 +703,7 @@ public class ControlLoopCompiler implements Serializable { } @Override - public String getID() { + public String getId() { return finalResult.toString(); } } diff --git a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/Policy.java b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/Policy.java index 22c58c8ab..06dc36e69 100644 --- a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/Policy.java +++ b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/Policy.java @@ -50,6 +50,93 @@ public class Policy implements Serializable { //Does Nothing Empty Constructor } + public Policy(String id) { + this.id = id; + } + + /** + * Constructor. + * + * @param name name + * @param actor actor + * @param recipe recipe + * @param payload payload + * @param target target + */ + public Policy(String name, String actor, String recipe, Map payload, Target target) { + this.name = name; + this.actor = actor; + this.recipe = recipe; + this.target = target; + if (payload != null) { + this.payload = Collections.unmodifiableMap(payload); + } + } + + /** + * Constructor. + * + * @param name name + * @param actor actor + * @param recipe recipe + * @param payload payload + * @param target target + * @param retries retries + * @param timeout timeout + */ + public Policy(String name, String actor, String recipe, Map payload, Target target, + Integer retries, Integer timeout) { + this(name, actor, recipe, payload, target); + this.retry = retries; + this.timeout = timeout; + } + + /** + * Constructor. + * + * @param id id + * @param name name + * @param description description + * @param actor actor + * @param payload payload + * @param target target + * @param recipe recipe + * @param retries retries + * @param timeout timeout + */ + public Policy(String id, String name, String description, String actor, Map payload, + Target target, String recipe, Integer retries, Integer timeout) { + this(name, actor, recipe, payload, target, retries, timeout); + this.id = id; + this.description = description; + } + + /** + * Constructor. + * + * @param policy copy object + */ + public Policy(Policy policy) { + this.id = policy.id; + this.name = policy.name; + this.description = policy.description; + this.actor = policy.actor; + this.recipe = policy.recipe; + if (policy.payload != null) { + this.payload = Collections.unmodifiableMap(policy.payload); + } + this.target = policy.target; + this.operationsAccumulateParams = policy.operationsAccumulateParams; + this.retry = policy.retry; + this.timeout = policy.timeout; + this.success = policy.success; + this.failure = policy.failure; + this.failureException = policy.failureException; + this.failureGuard = policy.failureGuard; + this.failureRetries = policy.failureRetries; + this.failureTimeout = policy.failureTimeout; + } + public String getId() { return id; } @@ -178,93 +265,6 @@ public class Policy implements Serializable { this.failureGuard = failureGuard; } - public Policy(String id) { - this.id = id; - } - - /** - * Constructor. - * - * @param name name - * @param actor actor - * @param recipe recipe - * @param payload payload - * @param target target - */ - public Policy(String name, String actor, String recipe, Map payload, Target target) { - this.name = name; - this.actor = actor; - this.recipe = recipe; - this.target = target; - if (payload != null) { - this.payload = Collections.unmodifiableMap(payload); - } - } - - /** - * Constructor. - * - * @param name name - * @param actor actor - * @param recipe recipe - * @param payload payload - * @param target target - * @param retries retries - * @param timeout timeout - */ - public Policy(String name, String actor, String recipe, Map payload, Target target, - Integer retries, Integer timeout) { - this(name, actor, recipe, payload, target); - this.retry = retries; - this.timeout = timeout; - } - - /** - * Constructor. - * - * @param id id - * @param name name - * @param description description - * @param actor actor - * @param payload payload - * @param target target - * @param recipe recipe - * @param retries retries - * @param timeout timeout - */ - public Policy(String id, String name, String description, String actor, Map payload, - Target target, String recipe, Integer retries, Integer timeout) { - this(name, actor, recipe, payload, target, retries, timeout); - this.id = id; - this.description = description; - } - - /** - * Constructor. - * - * @param policy copy object - */ - public Policy(Policy policy) { - this.id = policy.id; - this.name = policy.name; - this.description = policy.description; - this.actor = policy.actor; - this.recipe = policy.recipe; - if (policy.payload != null) { - this.payload = Collections.unmodifiableMap(policy.payload); - } - this.target = policy.target; - this.operationsAccumulateParams = policy.operationsAccumulateParams; - this.retry = policy.retry; - this.timeout = policy.timeout; - this.success = policy.success; - this.failure = policy.failure; - this.failureException = policy.failureException; - this.failureGuard = policy.failureGuard; - this.failureRetries = policy.failureRetries; - this.failureTimeout = policy.failureTimeout; - } - public boolean isValid() { return id != null && name != null && actor != null && recipe != null && target != null; } diff --git a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/Target.java b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/Target.java index 61d1ae285..eab6967f1 100644 --- a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/Target.java +++ b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/Target.java @@ -33,22 +33,6 @@ public class Target implements Serializable { //Does Nothing Empty Constructor } - public String getResourceID() { - return resourceId; - } - - public void setResourceID(String resourceId) { - this.resourceId = resourceId; - } - - public TargetType getType() { - return type; - } - - public void setType(TargetType type) { - this.type = type; - } - public Target(TargetType type) { this.type = type; } @@ -67,6 +51,22 @@ public class Target implements Serializable { this.resourceId = target.resourceId; } + public String getResourceID() { + return resourceId; + } + + public void setResourceID(String resourceId) { + this.resourceId = resourceId; + } + + public TargetType getType() { + return type; + } + + public void setType(TargetType type) { + this.type = type; + } + @Override public String toString() { return "Target [type=" + type + ", resourceId=" + resourceId + "]"; diff --git a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/builder/BuilderException.java b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/builder/BuilderException.java index 618678e9c..4038f3b74 100644 --- a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/builder/BuilderException.java +++ b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/builder/BuilderException.java @@ -22,10 +22,10 @@ package org.onap.policy.controlloop.policy.builder; public class BuilderException extends Exception { + private static final long serialVersionUID = 610064813684337895L; + public BuilderException(String string) { super(string); } - private static final long serialVersionUID = 610064813684337895L; - } diff --git a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/Constraint.java b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/Constraint.java index 5ac1fdc91..9332bfbe4 100644 --- a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/Constraint.java +++ b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/Constraint.java @@ -191,10 +191,24 @@ public class Constraint { this.maxVnfCount = maxVnfCount; } - + /** + * Check if these constraint values are valid. + * + * @return true if valid + */ public boolean isValid() { - return ((freqLimitPerTarget == null && timeWindow != null) - || (timeWindow == null && freqLimitPerTarget != null)) ? false : true; + // + // Sonar likes these statements combined as well as not use + // boolean literals. + // + // If the freqLimitPerTarget is null AND the timeWindow is NOT null + // OR + // timeWindow is null AND the freqLimitPerTarget is NOT null + // + // then we want to return false (hence the preceding !) + // + return ! ((freqLimitPerTarget == null && timeWindow != null) + || (timeWindow == null && freqLimitPerTarget != null)); } @Override diff --git a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/GuardPolicy.java b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/GuardPolicy.java index 799c5fcd7..b76c4578e 100644 --- a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/GuardPolicy.java +++ b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/GuardPolicy.java @@ -36,46 +36,6 @@ public class GuardPolicy { //Do Nothing Empty Constructor. } - public String getId() { - return id; - } - - public void setId(String id) { - this.id = id; - } - - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public String getDescription() { - return description; - } - - public void setDescription(String description) { - this.description = description; - } - - public MatchParameters getMatch_parameters() { - return matchParameters; - } - - public void setMatch_parameters(MatchParameters matchParameters) { - this.matchParameters = matchParameters; - } - - public LinkedList getLimit_constraints() { - return limitConstraints; - } - - public void setLimit_constraints(LinkedList limitConstraints) { - this.limitConstraints = limitConstraints; - } - public GuardPolicy(String id) { this.id = id; } @@ -140,6 +100,46 @@ public class GuardPolicy { } } + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public MatchParameters getMatch_parameters() { + return matchParameters; + } + + public void setMatch_parameters(MatchParameters matchParameters) { + this.matchParameters = matchParameters; + } + + public LinkedList getLimit_constraints() { + return limitConstraints; + } + + public void setLimit_constraints(LinkedList limitConstraints) { + this.limitConstraints = limitConstraints; + } + public boolean isValid() { return (id == null || name == null) ? false : true; } diff --git a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/MatchParameters.java b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/MatchParameters.java index 1bf10734d..0b1380972 100644 --- a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/MatchParameters.java +++ b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/MatchParameters.java @@ -34,38 +34,6 @@ public class MatchParameters { // Do Nothing Empty Constructor. } - public String getControlLoopName() { - return controlLoopName; - } - - public void setControlLoopName(String controlLoopName) { - this.controlLoopName = controlLoopName; - } - - public String getActor() { - return actor; - } - - public void setActor(String actor) { - this.actor = actor; - } - - public String getRecipe() { - return recipe; - } - - public void setRecipe(String recipe) { - this.recipe = recipe; - } - - public List getTargets() { - return targets; - } - - public void setTargets(List targets) { - this.targets = targets; - } - public MatchParameters(String actor, String recipe) { this.actor = actor; this.recipe = recipe; @@ -105,6 +73,38 @@ public class MatchParameters { } } + public String getControlLoopName() { + return controlLoopName; + } + + public void setControlLoopName(String controlLoopName) { + this.controlLoopName = controlLoopName; + } + + public String getActor() { + return actor; + } + + public void setActor(String actor) { + this.actor = actor; + } + + public String getRecipe() { + return recipe; + } + + public void setRecipe(String recipe) { + this.recipe = recipe; + } + + public List getTargets() { + return targets; + } + + public void setTargets(List targets) { + this.targets = targets; + } + @Override public String toString() { return "MatchParameters [controlLoopName=" + controlLoopName + ", actor=" + actor + ", recipe=" + recipe diff --git a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/builder/impl/ControlLoopGuardBuilderImpl.java b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/builder/impl/ControlLoopGuardBuilderImpl.java index f995ba4c6..da2b9a122 100644 --- a/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/builder/impl/ControlLoopGuardBuilderImpl.java +++ b/controlloop/common/policy-yaml/src/main/java/org/onap/policy/controlloop/policy/guard/builder/impl/ControlLoopGuardBuilderImpl.java @@ -235,7 +235,7 @@ public class ControlLoopGuardBuilderImpl implements ControlLoopGuardBuilder { try { ControlLoopGuardCompiler.compile(clGuard, callback); } catch (CompilerException e) { - logger.error(e.getMessage() + e); + logger.error("Build specification threw ", e); callback.results.addMessage(new MessageImpl(e.getMessage(), MessageLevel.EXCEPTION)); } // -- cgit 1.2.3-korg