From 01cbeec256bf5ec9e3ec2e7f599aef7eb4814ae3 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Tue, 17 Mar 2020 09:52:03 -0400 Subject: More sonar fixes in drools-apps Issue-ID: POLICY-2426 Signed-off-by: Jim Hahn Change-Id: Idfdcb229d05ee7f0220f44f8099284caaed754d4 --- .../common/rules/test/BaseRuleTest.java | 13 ++- .../controlloop/common/rules/test/Listener.java | 2 +- .../controlloop/common/rules/test/Rules.java | 28 ++++-- .../controlloop/common/rules/test/RulesTest.java | 107 ++++++++++++++++++--- .../rules-test/src/test/resources/logback-test.xml | 44 +++++++++ 5 files changed, 167 insertions(+), 27 deletions(-) create mode 100644 controlloop/common/rules-test/src/test/resources/logback-test.xml (limited to 'controlloop') 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 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 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 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 @@ -92,6 +108,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. */ @@ -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 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 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 @@ + + + + + + RulesTest + + + + + + %d %level %msg%n + + + + + + + + + + + + -- cgit 1.2.3-korg