Fix new sonars in drools-apps

Addressed the following sonars:
- too many assertions in test method
- rename test class
- use static method to modify static field
- use already defined constant
- code always returns the same value
- use assertNotSame
- use appropriate class name to access static method
- define a constant
- extract nested try block
- don't always return the same value
- use remove() instead of set(null) for thread-local-storage
- add @Override

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

4 files changed, 78 insertions, 54 deletions

diff --git a/controlloop/common/controller-tdjam/src/main/java/org/onap/policy/controlloop/tdjam/TdjamController.java b/controlloop/common/controller-tdjam/src/main/java/org/onap/policy/controlloop/tdjam/TdjamController.java
index 0b17f196f..3b3655122 100644
--- a/controlloop/common/controller-tdjam/src/main/java/org/onap/policy/controlloop/tdjam/TdjamController.java
+++ b/controlloop/common/controller-tdjam/src/main/java/org/onap/policy/controlloop/tdjam/TdjamController.java
@@ -24,6 +24,7 @@ import static org.onap.policy.drools.properties.DroolsPropertyConstants.PROPERTY
 
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -72,6 +73,9 @@ public class TdjamController extends NonDroolsPolicyController {
     // the 'controller.type' property is set to this value
     private static final String TDJAM_CONTROLLER_BUILDER_TAG = "tdjam";
 
+    // topic on which to publish event notifications
+    private static final String POLICY_CL_MGT = "POLICY-CL-MGT";
+
     // additional data associated with session
     private final String groupId;
     private final String artifactId;
@@ -352,7 +356,9 @@ public class TdjamController extends NonDroolsPolicyController {
                        cp.getClosedLoopControlName());
         }
 
-        logger.debug(new String(bos.toByteArray()));
+        if (logger.isDebugEnabled()) {
+            logger.debug(new String(bos.toByteArray(), StandardCharsets.UTF_8));
+        }
     }
 
     /**
@@ -454,7 +460,7 @@ public class TdjamController extends NonDroolsPolicyController {
         // Generate notification
         //
         try {
-            PolicyEngineConstants.getManager().deliver("POLICY-CL-MGT", notification);
+            PolicyEngineConstants.getManager().deliver(POLICY_CL_MGT, notification);
 
         } catch (RuntimeException e) {
             logger.warn("{}: {}.{}: event={} exception generating notification",
@@ -609,18 +615,14 @@ public class TdjamController extends NonDroolsPolicyController {
                 // we create the ControlLoopEventManager. The ControlLoopEventManager
                 // will do extra syntax checking as well as check if the closed loop is disabled.
                 //
-                try {
-                    start();
-                } catch (Exception e) {
-                    eventManagers.remove(requestId, this);
-                    onsetToEventManager.remove(event, this);
-                    throw e;
-                }
+                start();
                 notification = makeNotification();
                 notification.setNotification(ControlLoopNotificationType.ACTIVE);
                 notification.setPolicyName(params.getPolicyName() + "." + ruleName);
             } catch (Exception e) {
                 logger.warn("{}: {}.{}", clName, params.getPolicyName(), ruleName, e);
+                eventManagers.remove(requestId, this);
+                onsetToEventManager.remove(event, this);
                 notification = new VirtualControlLoopNotification(event);
                 notification.setNotification(ControlLoopNotificationType.REJECTED);
                 notification.setMessage("Exception occurred: " + e.getMessage());
@@ -632,7 +634,7 @@ public class TdjamController extends NonDroolsPolicyController {
             // Generate notification
             //
             try {
-                PolicyEngineConstants.getManager().deliver("POLICY-CL-MGT", notification);
+                PolicyEngineConstants.getManager().deliver(POLICY_CL_MGT, notification);
 
             } catch (RuntimeException e) {
                 logger.warn("{}: {}.{}: event={} exception generating notification",
@@ -719,7 +721,7 @@ public class TdjamController extends NonDroolsPolicyController {
             //
             try {
                 notification.setPolicyName(getPolicyName() + "." + ruleName);
-                PolicyEngineConstants.getManager().deliver("POLICY-CL-MGT", notification);
+                PolicyEngineConstants.getManager().deliver(POLICY_CL_MGT, notification);
 
             } catch (RuntimeException e) {
                 logger.warn("{}: {}.{}: manager={} exception generating notification",
@@ -766,7 +768,7 @@ public class TdjamController extends NonDroolsPolicyController {
             //
             try {
                 notification.setPolicyName(getPolicyName() + "." + ruleName);
-                PolicyEngineConstants.getManager().deliver("POLICY-CL-MGT", notification);
+                PolicyEngineConstants.getManager().deliver(POLICY_CL_MGT, notification);
             } catch (RuntimeException e) {
                 logger.warn("{}: {}.{}: manager={} exception generating notification",
                             getClosedLoopControlName(), getPolicyName(), ruleName,
@@ -825,7 +827,7 @@ public class TdjamController extends NonDroolsPolicyController {
                                       List<TopicCoderFilterConfiguration> encoderConfigurations) throws LinkageError {
 
             if (TDJAM_CONTROLLER_BUILDER_TAG.equals(properties.getProperty(PROPERTY_CONTROLLER_TYPE))) {
-                return TdjamController.getBuildInProgress();
+                return NonDroolsPolicyController.getBuildInProgress();
             }
             return null;
         }
diff --git a/controlloop/common/controller-tdjam/src/main/java/org/onap/policy/extension/system/NonDroolsPolicyController.java b/controlloop/common/controller-tdjam/src/main/java/org/onap/policy/extension/system/NonDroolsPolicyController.java
index d876bee96..97eb6a04d 100644
--- a/controlloop/common/controller-tdjam/src/main/java/org/onap/policy/extension/system/NonDroolsPolicyController.java
+++ b/controlloop/common/controller-tdjam/src/main/java/org/onap/policy/extension/system/NonDroolsPolicyController.java
@@ -143,12 +143,13 @@ public class NonDroolsPolicyController extends AggregatedPolicyController implem
         return buildInProgress.get();
     }
 
+    @Override
     protected void initDrools(Properties properties) {
         try {
             // Register with drools factory
             buildInProgress.set(this);
             this.droolsController.set(getDroolsFactory().build(properties, sources, sinks));
-            buildInProgress.set(null);
+            buildInProgress.remove();
         } catch (Exception | LinkageError e) {
             logger.error("{}: cannot init-drools", this);
             throw new IllegalArgumentException(e);
@@ -198,9 +199,6 @@ public class NonDroolsPolicyController extends AggregatedPolicyController implem
         logger.info("START: {}", this);
 
         synchronized (this) {
-            if (this.alive) {
-                return true;
-            }
             this.alive = true;
         }
 
@@ -213,9 +211,6 @@ public class NonDroolsPolicyController extends AggregatedPolicyController implem
         logger.info("STOP: {}", this);
 
         synchronized (this) {
-            if (!this.alive) {
-                return true;
-            }
             this.alive = false;
         }
 
diff --git a/controlloop/common/controller-tdjam/src/test/java/org/onap/policy/controlloop/tdjam/TdjamControllerTest.java b/controlloop/common/controller-tdjam/src/test/java/org/onap/policy/controlloop/tdjam/TdjamControllerTest.java
index 1426865c6..990e473db 100644
--- a/controlloop/common/controller-tdjam/src/test/java/org/onap/policy/controlloop/tdjam/TdjamControllerTest.java
+++ b/controlloop/common/controller-tdjam/src/test/java/org/onap/policy/controlloop/tdjam/TdjamControllerTest.java
@@ -22,6 +22,7 @@ package org.onap.policy.controlloop.tdjam;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
@@ -211,7 +212,7 @@ public class TdjamControllerTest {
         clp.setClosedLoopControlName(closedLoopControlName);
 
         if (tc != null) {
-            assertTrue(tc.addControlLoopParams(clp) != clp);
+            assertNotSame(clp, tc.addControlLoopParams(clp));
         }
 
         return clp;
diff --git a/controlloop/common/controller-tdjam/src/test/java/org/onap/policy/extension/system/NonDroolsPolicyControllerTest.java b/controlloop/common/controller-tdjam/src/test/java/org/onap/policy/extension/system/NonDroolsPolicyControllerTest.java
index ee96cb893..57f98bd0a 100644
--- a/controlloop/common/controller-tdjam/src/test/java/org/onap/policy/extension/system/NonDroolsPolicyControllerTest.java
+++ b/controlloop/common/controller-tdjam/src/test/java/org/onap/policy/extension/system/NonDroolsPolicyControllerTest.java
@@ -36,6 +36,8 @@ import static org.onap.policy.drools.properties.DroolsPropertyConstants.PROPERTY
 import java.util.List;
 import java.util.Properties;
 import java.util.function.Function;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.onap.policy.common.endpoints.event.comm.TopicSink;
@@ -52,15 +54,37 @@ public class NonDroolsPolicyControllerTest {
     //public static boolean loop = true;
     private static Properties prop;
 
+    private NonDroolsPolicyController controller;
+
     @BeforeClass
     public static void setupClass() throws Exception {
         prop = PropertyUtil.getProperties("src/test/resources/config/tdjam-controller.properties");
     }
 
+    /**
+     * Resets the stats and creates the controller.
+     */
+    @Before
+    public void setUp() {
+        DroolsControllerFeatureHandler.resetStats();
+
+        controller = buildController("tdjam");
+    }
+
+    /**
+     * Destroys the controller.
+     */
+    @After
+    public void tearDown() {
+        String name = controller.getName();
+        assertSame(controller, PolicyControllerConstants.getFactory().get(name));
+        PolicyControllerConstants.getFactory().destroy(controller);
+        assertThatIllegalArgumentException().isThrownBy(
+            () -> PolicyControllerConstants.getFactory().get(name));
+    }
+
     @Test
     public void testState() {
-        NonDroolsPolicyController controller = buildController("tdjam");
-
         assertEquals("nondrools", controller.getName());
         assertEquals("NonDroolsPolicyController", controller.getGroupId());
         assertEquals("nondrools", controller.getArtifactId());
@@ -81,48 +105,60 @@ public class NonDroolsPolicyControllerTest {
         assertFalse(controller.isLocked());
 
         // test a few stubbed-off methods
+        assertNull(controller.getContainer());
+        assertThatIllegalStateException().isThrownBy(() -> controller.updateToVersion(null, null, null, null, null));
+
+        controller.lock();
+        assertTrue(controller.isAlive());
+        assertTrue(controller.isLocked());
+
+        controller.stop();
+        assertFalse(controller.isAlive());
+        assertTrue(controller.isLocked());
+
+        controller.unlock();
+        assertFalse(controller.isAlive());
+        assertFalse(controller.isLocked());
+    }
+
+    @Test
+    public void testNames() {
         assertTrue(controller.getSessionNames().isEmpty());
         assertTrue(controller.getCanonicalSessionNames().isEmpty());
         assertTrue(controller.getBaseDomainNames().isEmpty());
+    }
+
+    @Test
+    public void testOffer() {
+        controller.start();
+
         assertFalse(controller.offer("topic", "event"));
         assertFalse(controller.offer("event"));
         assertEquals(0, controller.getRecentSourceEvents().length);
         assertEquals(0, controller.getRecentSinkEvents().length);
-        assertNull(controller.getContainer());
+    }
+
+    @Test
+    public void testFacts() {
         assertThatIllegalArgumentException().isThrownBy(
             () -> controller.fetchModelClass("NoSuchClass"));
-        assertThatIllegalStateException().isThrownBy(
-            () -> controller.updateToVersion(null, null, null, null, null));
         assertTrue(controller.factClassNames(null).isEmpty());
         assertEquals(0, controller.factCount(null));
         assertTrue(controller.facts(null, null, false).isEmpty());
         assertTrue(controller.facts("sessionName", String.class).isEmpty());
         assertTrue(controller.factQuery(null, null, null, false).isEmpty());
+    }
+
+    @Test
+    public void testDelete() {
         assertFalse(controller.delete("sessionName", "fact"));
         assertFalse(controller.delete("fact"));
         assertFalse(controller.delete("sessionName", String.class));
         assertFalse(controller.delete(String.class));
-
-        controller.lock();
-        assertTrue(controller.isAlive());
-        assertTrue(controller.isLocked());
-
-        controller.stop();
-        assertFalse(controller.isAlive());
-        assertTrue(controller.isLocked());
-
-        controller.unlock();
-        assertFalse(controller.isAlive());
-        assertFalse(controller.isLocked());
-
-        destroy(controller);
     }
 
     @Test
-    public void deliverTest() {
-        DroolsControllerFeatureHandler.resetStats();
-        final NonDroolsPolicyController controller = buildController("tdjam");
-
+    public void testDeliver() {
         final TopicSink topicSink = mock(TopicSink.class);
         when(topicSink.getTopic()).thenReturn("POLICY-CL-MGT");
         when(topicSink.send(any())).thenReturn(false);
@@ -201,8 +237,6 @@ public class NonDroolsPolicyControllerTest {
 
         assertFalse(signal.apply("nothing in particular"));
         assertEquals(1, DroolsControllerFeatureHandler.afterDeliverFalse);
-
-        destroy(controller);
     }
 
     private NonDroolsPolicyController buildController(String type) {
@@ -213,14 +247,6 @@ public class NonDroolsPolicyControllerTest {
         return (NonDroolsPolicyController) controller;
     }
 
-    private void destroy(PolicyController controller) {
-        String name = controller.getName();
-        assertSame(controller, PolicyControllerConstants.getFactory().get(name));
-        PolicyControllerConstants.getFactory().destroy(controller);
-        assertThatIllegalArgumentException().isThrownBy(
-            () -> PolicyControllerConstants.getFactory().get(name));
-    }
-
     /* ============================================================ */
 
     /**