Merge "Fix more sonar issues"

9 files changed, 237 insertions, 205 deletions

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 @@
  
 <suppressions>
   <suppress checks="AbbreviationAsWordInName"
-    files="ControlLoopCompiler.java|Target.java|ControlLoopPolicyBuilder.java"
+    files="Target.java|ControlLoopPolicyBuilder.java"
     lines="1-9999"/>
 </suppressions>
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<NodeWrapper, LabeledEdge> 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<NodeWrapper, LabeledEdge> 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<NodeWrapper, LabeledEdge> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<Constraint> getLimit_constraints() {
-        return  limitConstraints;
-    }
-
-    public void setLimit_constraints(LinkedList<Constraint> 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<Constraint> getLimit_constraints() {
+        return  limitConstraints;
+    }
+
+    public void setLimit_constraints(LinkedList<Constraint> 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<String> getTargets() {
-        return targets;
-    }
-
-    public void setTargets(List<String> 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<String> getTargets() {
+        return targets;
+    }
+
+    public void setTargets(List<String> 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));
         }
         //