Address sonar issues in drools-pdp

Addressed the following sonar issues:
- add "final" to public static fields
- commented code; some were bogus - just updated the comments
  so sonar is happy
- use "{}" instead of string concatenation
- junit should assert something
- when using logger, invoke compute-intensive tasks conditionally
- use superclass name instead of subclass name to access static fields
- don't always return the same value
- remove "transient" from fields of classes that aren't Serializable
- don't nest try/catch blocks
- use appropriate class name in Logger.getLogger()
- use Predicate<T> instead of Function<T,Boolean>
- remove unused parameters from private methods
- either log or throw
- remove duplicate methods
- use remove() TLS instead of set(null)
- null check is implicit in instanceof check
- do something with return value
- don't use volatile
- don't return "null" list; used Optional instead
- add no-arg constructor to non-Serializable superclass
- add callSuper=true for EqualsAndHashCode
- don't declare "throws XXX" where XXX is a subclass of RuntimeException
- remove serialVersionUID field if the class isn't Serializable

Also addressed some eclipse warnings:
- unused fields
- suppress generic typic cast warnings

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

7 files changed, 30 insertions, 27 deletions

diff --git a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/LifecycleFeature.java b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/LifecycleFeature.java
index 12828e02..003740c1 100644
--- a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/LifecycleFeature.java
+++ b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/LifecycleFeature.java
@@ -71,7 +71,7 @@ public class LifecycleFeature
 
     @Override
     public boolean beforeShutdown(PolicyEngine engine) {
-        return fsmShutdown(engine);
+        return fsmShutdown();
     }
 
     @Override
@@ -114,7 +114,7 @@ public class LifecycleFeature
         return false;
     }
 
-    private boolean fsmShutdown(PolicyEngine engine) {
+    private boolean fsmShutdown() {
         fsm.shutdown();
         return false;
     }
diff --git a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsController.java b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsController.java
index 8dfbf2f3..71e40726 100644
--- a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsController.java
+++ b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsController.java
@@ -24,7 +24,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Function;
+import java.util.function.Predicate;
 import lombok.Getter;
 import lombok.NonNull;
 import org.apache.commons.lang3.StringUtils;
@@ -50,7 +50,7 @@ public class PolicyTypeDroolsController implements PolicyTypeController {
     protected static final ToscaPolicyTypeIdentifier compliantType =
         new ToscaPolicyTypeIdentifier("onap.policies.controlloop.operational.common.Drools", "1.0.0");
 
-    private static final Logger logger = LoggerFactory.getLogger(PolicyTypeController.class);
+    private static final Logger logger = LoggerFactory.getLogger(PolicyTypeDroolsController.class);
 
     @Getter
     protected final Map<String, PolicyController> controllers = new ConcurrentHashMap<>();
@@ -60,7 +60,7 @@ public class PolicyTypeDroolsController implements PolicyTypeController {
 
     @GsonJsonIgnore
     @JsonIgnore
-    protected final transient LifecycleFsm fsm;
+    protected final LifecycleFsm fsm;
 
     /**
      * Creates a Policy Type Drools Controller.
@@ -120,17 +120,12 @@ public class PolicyTypeDroolsController implements PolicyTypeController {
         return List.of(this.controllers.get(controllerName));
     }
 
-    private boolean perform(ToscaPolicy policy, Function<PolicyController, Boolean> operation) {
+    private boolean perform(ToscaPolicy policy, Predicate<PolicyController> operation) {
         try {
             List<PolicyController> selected = selectControllers(policy);
             boolean success = true;
             for (PolicyController controller : selected) {
-                try {
-                    success = operation.apply(controller) && success;
-                } catch (RuntimeException r) {
-                    logger.warn("invalid offer to controller: {}", controller);
-                    success = false;
-                }
+                success = modifyController(operation, controller) && success;
             }
             return success && !selected.isEmpty();
         } catch (CoderException e) {
@@ -139,6 +134,15 @@ public class PolicyTypeDroolsController implements PolicyTypeController {
         }
     }
 
+    private boolean modifyController(Predicate<PolicyController> operation, PolicyController controller) {
+        try {
+            return operation.test(controller);
+        } catch (RuntimeException r) {
+            logger.warn("invalid offer to controller: {}", controller);
+            return false;
+        }
+    }
+
     private List<PolicyController> selectControllers(ToscaPolicy policy) throws CoderException {
         List<PolicyController> selected;
         if (legacyType.equals(policyType)) {
diff --git a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeArtifactController.java b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeArtifactController.java
index 113bb885..810c2ecf 100644
--- a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeArtifactController.java
+++ b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeArtifactController.java
@@ -44,7 +44,7 @@ public class PolicyTypeNativeArtifactController implements PolicyTypeController
 
     @GsonJsonIgnore
     @JsonIgnore
-    protected final transient LifecycleFsm fsm;
+    protected final LifecycleFsm fsm;
 
     public PolicyTypeNativeArtifactController(LifecycleFsm fsm, ToscaPolicyTypeIdentifier policyType) {
         this.policyType = policyType;
diff --git a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeDroolsController.java b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeDroolsController.java
index e74b2895..9a11955a 100644
--- a/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeDroolsController.java
+++ b/feature-lifecycle/src/main/java/org/onap/policy/drools/lifecycle/PolicyTypeNativeDroolsController.java
@@ -55,7 +55,7 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController {
 
     @GsonJsonIgnore
     @JsonIgnore
-    protected final transient LifecycleFsm fsm;
+    protected final LifecycleFsm fsm;
 
     public PolicyTypeNativeDroolsController(LifecycleFsm fsm, ToscaPolicyTypeIdentifier policyType) {
         this.policyType = policyType;
@@ -72,9 +72,10 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController {
 
         ControllerProperties controllerConfig = controllerPolicy.getProperties();
 
+        configControllerName(controllerConfig, controllerProps);
+
         boolean success =
-            configControllerName(controllerConfig, controllerProps)
-            && configControllerSources(controllerConfig, controllerProps)
+            configControllerSources(controllerConfig, controllerProps)
             && configControllerSinks(controllerConfig, controllerProps)
             && configControllerCustom(controllerConfig, controllerProps);
 
@@ -145,10 +146,9 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController {
         return nativePolicy;
     }
 
-    private boolean configControllerName(ControllerProperties controllerConfig, Properties controllerProps)  {
+    private void configControllerName(ControllerProperties controllerConfig, Properties controllerProps)  {
         controllerProps
                 .setProperty(DroolsPropertyConstants.PROPERTY_CONTROLLER_NAME, controllerConfig.getControllerName());
-        return true;
     }
 
     private boolean configControllerSources(ControllerProperties controllerConfig, Properties controllerProps) {
@@ -241,11 +241,10 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController {
 
     private boolean configControllerCustom(ControllerProperties controllerConfig, Properties controllerProps) {
         Map<String, String> configCustom = controllerConfig.getCustomConfig();
-        if (configCustom == null || configCustom.isEmpty()) {
-            return true;
+        if (configCustom != null && !configCustom.isEmpty()) {
+            controllerProps.putAll(configCustom);
         }
 
-        controllerProps.putAll(configCustom);
         return true;
     }
 
@@ -255,7 +254,7 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController {
 
     private List<String> sourceTopics(List<ControllerSourceTopic> sourceTopics) {
         if (sourceTopics == null) {
-            return Collections.EMPTY_LIST;
+            return Collections.emptyList();
         }
 
         return sourceTopics.stream()
@@ -265,7 +264,7 @@ public class PolicyTypeNativeDroolsController implements PolicyTypeController {
 
     private List<String> sinkTopics(List<ControllerSinkTopic> sinkTopics) {
         if (sinkTopics == null) {
-            return Collections.EMPTY_LIST;
+            return Collections.emptyList();
         }
 
         return sinkTopics.stream()
diff --git a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActivePoliciesTest.java b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActivePoliciesTest.java
index 0e5937ff..fdcaac50 100644
--- a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActivePoliciesTest.java
+++ b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActivePoliciesTest.java
@@ -125,6 +125,7 @@ public class LifecycleStateActivePoliciesTest extends LifecycleStateRunningTest
         ToscaPolicy policyNativeArtifact =
                 getPolicyFromFile(EXAMPLE_NATIVE_ARTIFACT_POLICY_JSON, EXAMPLE_NATIVE_DROOLS_ARTIFACT_POLICY_NAME);
 
+        @SuppressWarnings("unchecked")
         Map<String, String> controllerMap =
                 (Map<String, String>) policyNativeArtifact.getProperties().get("controller");
         controllerMap.put("name", "xyz987");
@@ -239,7 +240,7 @@ public class LifecycleStateActivePoliciesTest extends LifecycleStateRunningTest
         assertEquals(policyNativeController, fsm.getPoliciesMap().get(policyNativeController.getIdentifier()));
         assertEquals(policyNativeFooController, fsm.getPoliciesMap().get(policyNativeFooController.getIdentifier()));
 
-        update.setPolicies(Collections.EMPTY_LIST);
+        update.setPolicies(Collections.emptyList());
         assertTrue(fsm.update(update));
         assertThatIllegalArgumentException().isThrownBy(() -> controllerSupport.getController().getDrools());
         assertNull(fsm.getPoliciesMap().get(policyNativeController.getIdentifier()));
diff --git a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActiveTest.java b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActiveTest.java
index f370d0db..54f7d68f 100644
--- a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActiveTest.java
+++ b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateActiveTest.java
@@ -155,7 +155,7 @@ public class LifecycleStateActiveTest extends LifecycleStateRunningTest {
 
         fsm.source.offer(new StandardCoder().encode(change));
         assertEquals(PdpState.ACTIVE, fsm.state());
-        assertEquals(LifecycleFsm.DEFAULT_PDP_GROUP, fsm.getGroup());;
+        assertEquals(LifecycleFsm.DEFAULT_PDP_GROUP, fsm.getGroup());
         assertNotEquals("b", fsm.getSubgroup());
 
         change.setName(fsm.getName());
diff --git a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsControllerTest.java b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsControllerTest.java
index a56e250b..bb4b5638 100644
--- a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsControllerTest.java
+++ b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/PolicyTypeDroolsControllerTest.java
@@ -45,7 +45,6 @@ public class PolicyTypeDroolsControllerTest extends LifecycleStateRunningTest {
             "policies/vCPE.policy.operational.input.tosca.json";
 
     private ToscaPolicy policy;
-    private OperationalPolicy operationalPolicy;
     private PolicyTypeDroolsController controller;
 
     /**
@@ -55,7 +54,7 @@ public class PolicyTypeDroolsControllerTest extends LifecycleStateRunningTest {
     public void init() throws CoderException {
         fsm = makeFsmWithPseudoTime();
         policy = getExamplesPolicy(VCPE_OPERATIONAL_DROOLS_POLICY_JSON, OP_POLICY_NAME_VCPE);
-        operationalPolicy = fsm.getDomainMaker().convertTo(policy, OperationalPolicy.class);
+        fsm.getDomainMaker().convertTo(policy, OperationalPolicy.class);
         controller = new PolicyTypeDroolsController(
             fsm, PolicyTypeDroolsController.compliantType, controllerSupport.getController());