More sonar fixes in drools-apps

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

5 files changed, 167 insertions, 27 deletions

diff --git a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTest.java b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTest.java
index 711a61738..1de02e54c 100644
--- a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTest.java
+++ b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseRuleTest.java
@@ -44,6 +44,8 @@ import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
  * Superclass used for rule tests.
  */
 public abstract class BaseRuleTest {
+    private static final String APPC_RESTART_OP = "restart";
+
     /*
      * Canonical Topic Names.
      */
@@ -185,7 +187,7 @@ public abstract class BaseRuleTest {
 
         // restart request should be sent and fail four times (i.e., because retry=3)
         for (int count = 0; count < 4; ++count) {
-            AppcLcmDmaapWrapper appcreq = appcLcmRead.await(req -> "restart".equals(req.getRpcName()));
+            AppcLcmDmaapWrapper appcreq = appcLcmRead.await(req -> APPC_RESTART_OP.equals(req.getRpcName()));
 
             topics.inject(APPC_LCM_WRITE_TOPIC, SERVICE123_APPC_RESTART_FAILURE,
                             appcreq.getBody().getInput().getCommonHeader().getSubRequestId());
@@ -245,7 +247,7 @@ public abstract class BaseRuleTest {
 
         // should see two restarts
         for (int count = 0; count < 2; ++count) {
-            AppcLcmDmaapWrapper appcreq = appcLcmRead.await(req -> "restart".equals(req.getRpcName()));
+            AppcLcmDmaapWrapper appcreq = appcLcmRead.await(req -> APPC_RESTART_OP.equals(req.getRpcName()));
 
             // indicate success
             topics.inject(APPC_LCM_WRITE_TOPIC, DUPLICATES_APPC_SUCCESS,
@@ -270,7 +272,7 @@ public abstract class BaseRuleTest {
      */
     @Test
     public void testVcpeSunnyDayLegacy() {
-        appcLcmSunnyDay(VCPE_TOSCA_LEGACY_POLICY, VCPE_ONSET_1, "restart");
+        appcLcmSunnyDay(VCPE_TOSCA_LEGACY_POLICY, VCPE_ONSET_1, APPC_RESTART_OP);
     }
 
     /**
@@ -278,7 +280,7 @@ public abstract class BaseRuleTest {
      */
     @Test
     public void testVcpeSunnyDayCompliant() {
-        appcLcmSunnyDay(VCPE_TOSCA_COMPLIANT_POLICY, VCPE_ONSET_1, "restart");
+        appcLcmSunnyDay(VCPE_TOSCA_COMPLIANT_POLICY, VCPE_ONSET_1, APPC_RESTART_OP);
     }
 
     /**
@@ -288,7 +290,8 @@ public abstract class BaseRuleTest {
      */
     @Test
     public void testVcpeOnsetFloodPrevention() {
-        appcLcmSunnyDay(VCPE_TOSCA_COMPLIANT_POLICY, List.of(VCPE_ONSET_1, VCPE_ONSET_2, VCPE_ONSET_3), "restart");
+        appcLcmSunnyDay(VCPE_TOSCA_COMPLIANT_POLICY, List.of(VCPE_ONSET_1, VCPE_ONSET_2, VCPE_ONSET_3),
+                        APPC_RESTART_OP);
     }
 
     // VDNS
diff --git a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/Listener.java b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/Listener.java
index 5042f54c6..a353c98fb 100644
--- a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/Listener.java
+++ b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/Listener.java
@@ -41,7 +41,7 @@ import org.slf4j.LoggerFactory;
  */
 public class Listener<T> implements TopicListener {
     private static final Logger logger = LoggerFactory.getLogger(Listener.class);
-    public static long DEFAULT_WAIT_SEC = 5L;
+    private static final long DEFAULT_WAIT_SEC = 5L;
 
     private final TopicSink sink;
     private final Function<String, T> decoder;
diff --git a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/Rules.java b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/Rules.java
index 7549c6be6..2e15895bc 100644
--- a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/Rules.java
+++ b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/Rules.java
@@ -74,6 +74,7 @@ import org.slf4j.LoggerFactory;
 public class Rules {
     private static final Logger logger = LoggerFactory.getLogger(Rules.class);
     private static final StandardCoder coder = new StandardCoder();
+    private static final String POLICY_MSG = "policy ";
 
     /**
      * PDP-D Engine.
@@ -116,7 +117,7 @@ public class Rules {
             File kmoduleFile = new File(resourceDir + "/META-INF/kmodule.xml");
             File pomFile = new File("src/test/resources/" + controllerName + ".pom");
             String resourceDir2 = resourceDir + "/org/onap/policy/controlloop/";
-            File ruleFile = new File(resourceDir + "/" + controllerName + ".drl");
+            File ruleFile = new File(resourceDir + File.separator + controllerName + ".drl");
             List<File> ruleFiles = Collections.singletonList(ruleFile);
 
             installArtifact(kmoduleFile, pomFile, resourceDir2, ruleFiles);
@@ -181,8 +182,12 @@ public class Rules {
         try {
             return setupPolicy(getPolicyFromTemplate(templatePath, policyName));
 
-        } catch (InterruptedException | CoderException e) {
-            throw new IllegalArgumentException("policy " + policyName, e);
+        } catch (CoderException e) {
+            throw new IllegalArgumentException(POLICY_MSG + policyName, e);
+
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new IllegalArgumentException(POLICY_MSG + policyName, e);
         }
     }
 
@@ -218,12 +223,16 @@ public class Rules {
         try {
             return setupPolicy(getPolicyFromFile(policyPath));
 
-        } catch (InterruptedException | IOException | CoderException e) {
-            throw new IllegalArgumentException("policy " + policyPath, e);
+        } catch (CoderException e) {
+            throw new IllegalArgumentException(POLICY_MSG + policyPath, e);
+
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new IllegalArgumentException(POLICY_MSG + policyPath, e);
         }
     }
 
-    private ToscaPolicy getPolicyFromFile(String policyPath) throws IOException, CoderException {
+    private ToscaPolicy getPolicyFromFile(String policyPath) throws CoderException {
         String policyJson = ResourceUtils.getResourceAsString(policyPath);
         if (policyJson == null) {
             throw new CoderException(new FileNotFoundException(policyPath));
@@ -232,7 +241,7 @@ public class Rules {
         return coder.decode(policyJson, ToscaPolicy.class);
     }
 
-    private ToscaPolicy setupPolicy(ToscaPolicy policy) throws InterruptedException {
+    protected ToscaPolicy setupPolicy(ToscaPolicy policy) throws InterruptedException {
         final KieObjectExpectedCallback<?> policyTracker = new KieObjectInsertedExpectedCallback<>(policy);
         final KieObjectExpectedCallback<?> paramsTracker =
                         new KieClassInsertedExpectedCallback<>(ControlLoopParams.class);
@@ -243,10 +252,10 @@ public class Rules {
         assertTrue(paramsTracker.isNotified());
 
         assertEquals(1, controller.getDrools().facts(controllerName, ToscaPolicy.class).stream()
-                        .filter((anotherPolicy) -> anotherPolicy == policy).count());
+                        .filter(anotherPolicy -> anotherPolicy == policy).count());
 
         assertEquals(1, controller.getDrools().facts(controllerName, ControlLoopParams.class).stream()
-                        .filter((params) -> params.getToscaPolicy() == policy).count());
+                        .filter(params -> params.getToscaPolicy() == policy).count());
         return policy;
     }
 
@@ -377,6 +386,7 @@ public class Rules {
             super(affected);
         }
 
+        @Override
         public void objectInserted(ObjectInsertedEvent event) {
             if (subject == event.getObject().getClass()) {
                 callbacked();
diff --git a/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/RulesTest.java b/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/RulesTest.java
index 28cb977fc..89b1a1b69 100644
--- a/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/RulesTest.java
+++ b/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/RulesTest.java
@@ -20,6 +20,7 @@
 
 package org.onap.policy.controlloop.common.rules.test;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -32,12 +33,18 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import ch.qos.logback.classic.Logger;
 import java.io.File;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Properties;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
+import org.junit.AfterClass;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.kie.api.definition.rule.Rule;
 import org.kie.api.event.rule.AfterMatchFiredEvent;
@@ -53,6 +60,7 @@ import org.kie.api.runtime.KieSession;
 import org.kie.api.runtime.rule.Match;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
+import org.onap.policy.common.utils.test.log.logback.ExtractAppender;
 import org.onap.policy.controlloop.ControlLoopEvent;
 import org.onap.policy.controlloop.drl.legacy.ControlLoopParams;
 import org.onap.policy.controlloop.eventmanager.ControlLoopEventManager2;
@@ -62,8 +70,10 @@ import org.onap.policy.drools.system.PolicyController;
 import org.onap.policy.drools.system.PolicyControllerFactory;
 import org.onap.policy.drools.system.PolicyEngine;
 import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
+import org.slf4j.LoggerFactory;
 
 public class RulesTest {
+    private static final String EXPECTED_EXCEPTION = "expected exception";
     private static final String CONTROLLER_NAME = "rulesTest";
     private static final String POLICY_FILE = "src/test/resources/tosca-policy.json";
     private static final String MY_POLICY = "operational.restart";
@@ -71,6 +81,12 @@ public class RulesTest {
     private static final String MY_RULE_NAME = "my-rule-name";
     private static final String MY_TEXT = "my text";
 
+    /**
+     * Used to attach an appender to the class' logger.
+     */
+    private static final Logger logger = (Logger) LoggerFactory.getLogger(Rules.class);
+    private static final ExtractAppender appender = new ExtractAppender();
+
     @Mock
     private PolicyEngine engine;
     @Mock
@@ -93,6 +109,28 @@ public class RulesTest {
     private Rules rules;
 
     /**
+     * Attaches the appender to the logger.
+     */
+    @BeforeClass
+    public static void setUpBeforeClass() throws Exception {
+        /**
+         * Attach appender to the logger.
+         */
+        appender.setContext(logger.getLoggerContext());
+        appender.start();
+
+        logger.addAppender(appender);
+    }
+
+    /**
+     * Stops the appender.
+     */
+    @AfterClass
+    public static void tearDownAfterClass() {
+        appender.stop();
+    }
+
+    /**
      * Sets up.
      */
     @Before
@@ -206,18 +244,25 @@ public class RulesTest {
     }
 
     @Test
-    public void testSetupPolicyFromTemplate_testGetPolicyFromTemplate() {
+    public void testSetupPolicyFromTemplate_testGetPolicyFromTemplate() throws InterruptedException {
         rules.setupPolicyFromTemplate("tosca-template.json", MY_POLICY);
 
         assertThatIllegalArgumentException()
                         .isThrownBy(() -> rules.setupPolicyFromTemplate("missing-file.json", "a-policy"));
+
+        // check interrupt case
+        checkInterrupt(() -> rules.setupPolicyFromTemplate("tosca-template.json", MY_POLICY),
+                        "policy operational.restart");
     }
 
     @Test
-    public void testSetupPolicyFromFile_testGetPolicyFromFile_testSetupPolicy() {
+    public void testSetupPolicyFromFile_testGetPolicyFromFile_testSetupPolicy() throws InterruptedException {
         assertNotNull(rules.setupPolicyFromFile(POLICY_FILE));
 
         assertThatIllegalArgumentException().isThrownBy(() -> rules.setupPolicyFromFile("missing-file.json"));
+
+        // check interrupt case
+        checkInterrupt(() -> rules.setupPolicyFromFile(POLICY_FILE), "policy " + POLICY_FILE);
     }
 
     @Test
@@ -228,23 +273,23 @@ public class RulesTest {
         // insertions - with and without rule name
         ObjectInsertedEvent insert = mock(ObjectInsertedEvent.class);
         when(insert.getObject()).thenReturn(MY_TEXT);
-        ruleListeners.forEach(listener -> listener.objectInserted(insert));
+        checkLogging("inserted", () -> ruleListeners.forEach(listener -> listener.objectInserted(insert)));
         when(insert.getRule()).thenReturn(rule);
-        ruleListeners.forEach(listener -> listener.objectInserted(insert));
+        checkLogging("inserted", () -> ruleListeners.forEach(listener -> listener.objectInserted(insert)));
 
         // updates - with and without rule name
         ObjectUpdatedEvent update = mock(ObjectUpdatedEvent.class);
         when(update.getObject()).thenReturn(MY_TEXT);
-        ruleListeners.forEach(listener -> listener.objectUpdated(update));
+        checkLogging("updated", () -> ruleListeners.forEach(listener -> listener.objectUpdated(update)));
         when(update.getRule()).thenReturn(rule);
-        ruleListeners.forEach(listener -> listener.objectUpdated(update));
+        checkLogging("updated", () -> ruleListeners.forEach(listener -> listener.objectUpdated(update)));
 
         // deletions - with and without rule name
         ObjectDeletedEvent delete = mock(ObjectDeletedEvent.class);
         when(delete.getOldObject()).thenReturn(MY_TEXT);
-        ruleListeners.forEach(listener -> listener.objectDeleted(delete));
+        checkLogging("deleted", () -> ruleListeners.forEach(listener -> listener.objectDeleted(delete)));
         when(delete.getRule()).thenReturn(rule);
-        ruleListeners.forEach(listener -> listener.objectDeleted(delete));
+        checkLogging("deleted", () -> ruleListeners.forEach(listener -> listener.objectDeleted(delete)));
     }
 
     @Test
@@ -258,22 +303,25 @@ public class RulesTest {
         // create
         MatchCreatedEvent create = mock(MatchCreatedEvent.class);
         when(create.getMatch()).thenReturn(match);
-        agendaListeners.forEach(listener -> listener.matchCreated(create));
+        checkLogging("match created", () -> agendaListeners.forEach(listener -> listener.matchCreated(create)));
 
         // cancel
         MatchCancelledEvent cancel = mock(MatchCancelledEvent.class);
         when(cancel.getMatch()).thenReturn(match);
-        agendaListeners.forEach(listener -> listener.matchCancelled(cancel));
+        checkLogging("match cancelled", () -> agendaListeners.forEach(listener -> listener.matchCancelled(cancel)));
 
         // before-fire
         BeforeMatchFiredEvent before = mock(BeforeMatchFiredEvent.class);
         when(before.getMatch()).thenReturn(match);
-        agendaListeners.forEach(listener -> listener.beforeMatchFired(before));
+        // @formatter:off
+        checkLogging("before match fired",
+            () -> agendaListeners.forEach(listener -> listener.beforeMatchFired(before)));
+        // @formatter:on
 
         // after-fire
         AfterMatchFiredEvent after = mock(AfterMatchFiredEvent.class);
         when(after.getMatch()).thenReturn(match);
-        agendaListeners.forEach(listener -> listener.afterMatchFired(after));
+        checkLogging("after match fired", () -> agendaListeners.forEach(listener -> listener.afterMatchFired(after)));
     }
 
     @Test
@@ -285,6 +333,41 @@ public class RulesTest {
         assertNotNull(rules.getPdpdRepo());
     }
 
+    private void checkInterrupt(Runnable command, String expectedMsg) throws InterruptedException {
+        rules = new MyRules() {
+            @Override
+            protected ToscaPolicy setupPolicy(ToscaPolicy policy) throws InterruptedException {
+                throw new InterruptedException(EXPECTED_EXCEPTION);
+            }
+        };
+        rules.configure(RESOURCE_DIR);
+        rules.start();
+
+        BlockingQueue<IllegalArgumentException> exceptions = new LinkedBlockingQueue<>();
+
+        Thread thread = new Thread(() -> {
+            try {
+                command.run();
+            } catch (IllegalArgumentException e) {
+                exceptions.add(e);
+            }
+        });
+
+        thread.setDaemon(true);
+        thread.start();
+
+        assertThat(exceptions.poll(10, TimeUnit.SECONDS)).isNotNull().hasMessage(expectedMsg)
+                        .hasCauseInstanceOf(InterruptedException.class);
+    }
+
+    private void checkLogging(String expectedMsg, Runnable command) {
+        appender.clearExtractions();
+        command.run();
+        List<String> messages = appender.getExtracted();
+        assertEquals(1, messages.size());
+        assertThat(messages.get(0)).contains(expectedMsg);
+    }
+
     protected void notifyInserted(Object object) {
         // add it to our list
         facts.add(object);
diff --git a/controlloop/common/rules-test/src/test/resources/logback-test.xml b/controlloop/common/rules-test/src/test/resources/logback-test.xml
new file mode 100644
index 000000000..6b09022be
--- /dev/null
+++ b/controlloop/common/rules-test/src/test/resources/logback-test.xml
@@ -0,0 +1,44 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ============LICENSE_START=======================================================
+  ONAP
+  ================================================================================
+  Copyright (C) 2020 AT&T Intellectual Property. All rights reserved.
+  ================================================================================
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+  ============LICENSE_END=========================================================
+-->
+
+<configuration>
+
+    <contextName>RulesTest</contextName>
+    <statusListener class="ch.qos.logback.core.status.OnConsoleStatusListener" />
+
+    <!-- USE FOR STD OUT ONLY -->
+    <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+        <encoder>
+            <Pattern>%d %level  %msg%n</Pattern>
+        </encoder>
+    </appender>
+
+    <root level="warn">
+        <appender-ref ref="STDOUT" />
+    </root>
+
+    <!-- this is required for RulesTest -->
+    <logger
+            name="org.onap.policy.controlloop.common.rules.test.Rules"
+            level="info" additivity="false">
+        <appender-ref ref="STDOUT" />
+    </logger>
+</configuration>