summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJim Hahn <jrh3@att.com>2020-09-30 12:52:25 -0400
committerJim Hahn <jrh3@att.com>2020-09-30 14:59:39 -0400
commitfb9bd637567096f6174bbc2e52a5e149a4eed882 (patch)
tree7b0d2df1aefbb3635788af501be1157fbe05d798
parente8e477ab80c6762fb05aebfe9becc630d2d51e39 (diff)
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>
-rw-r--r--controlloop/common/controller-tdjam/pom.xml1
-rw-r--r--controlloop/common/controller-tdjam/src/main/java/org/onap/policy/controlloop/tdjam/TdjamController.java28
-rw-r--r--controlloop/common/controller-tdjam/src/main/java/org/onap/policy/extension/system/NonDroolsPolicyController.java9
-rw-r--r--controlloop/common/controller-tdjam/src/test/java/org/onap/policy/controlloop/tdjam/TdjamControllerTest.java3
-rw-r--r--controlloop/common/controller-tdjam/src/test/java/org/onap/policy/extension/system/NonDroolsPolicyControllerTest.java92
-rw-r--r--controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager2Drools.java2
-rw-r--r--controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManager2.java7
-rw-r--r--controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/Step.java7
-rw-r--r--controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseTest.java11
-rw-r--r--controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunner2Test.java (renamed from controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunnerTest2.java)2
10 files changed, 101 insertions, 61 deletions
diff --git a/controlloop/common/controller-tdjam/pom.xml b/controlloop/common/controller-tdjam/pom.xml
index bee07cb2b..af882e3ed 100644
--- a/controlloop/common/controller-tdjam/pom.xml
+++ b/controlloop/common/controller-tdjam/pom.xml
@@ -168,7 +168,6 @@
<artifactId>policy-management</artifactId>
<version>${version.policy.drools-pdp}</version>
<scope>provided</scope>
- <!-- <optional>true</optional> -->
</dependency>
<dependency>
<groupId>org.onap.policy.drools-applications.controlloop.common</groupId>
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));
- }
-
/* ============================================================ */
/**
diff --git a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager2Drools.java b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager2Drools.java
index 3af9defc7..9e8af9afb 100644
--- a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager2Drools.java
+++ b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager2Drools.java
@@ -33,6 +33,8 @@ import org.onap.policy.controlloop.drl.legacy.ControlLoopParams;
* {@link #isActive()} returns {@code false}, indicating that all steps have completed.
*/
public class ControlLoopEventManager2Drools extends ControlLoopEventManager2 {
+ private static final long serialVersionUID = 1L;
+
private final transient WorkingMemory workMem;
private transient FactHandle factHandle;
diff --git a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManager2.java b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManager2.java
index 31402f4cf..62d4fc8c0 100644
--- a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManager2.java
+++ b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManager2.java
@@ -300,7 +300,12 @@ public class ControlLoopOperationManager2 implements Serializable {
* @param thrown exception that was generated
* @return {@code null}
*/
- private OperationOutcome handleException(Throwable thrown) {
+ private OperationOutcome handleException(Throwable thrown) { // NOSONAR
+ /*
+ * disabling sonar about returning the same value because we prefer the code to be
+ * structured this way
+ */
+
if (thrown instanceof CancellationException || thrown.getCause() instanceof CancellationException) {
return null;
}
diff --git a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/Step.java b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/Step.java
index ae51c737f..1cbdb53b2 100644
--- a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/Step.java
+++ b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/Step.java
@@ -178,7 +178,12 @@ public class Step {
* @param thrown exception that was generated
* @return {@code null}
*/
- private OperationOutcome handleException(Throwable thrown) {
+ private OperationOutcome handleException(Throwable thrown) { // NOSONAR
+ /*
+ * disabling sonar about returning the same value because we prefer the code to be
+ * structured this way
+ */
+
if (thrown instanceof CancellationException || thrown.getCause() instanceof CancellationException) {
return null;
}
diff --git a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseTest.java b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseTest.java
index f5346def8..83825bca9 100644
--- a/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseTest.java
+++ b/controlloop/common/rules-test/src/main/java/org/onap/policy/controlloop/common/rules/test/BaseTest.java
@@ -30,6 +30,7 @@ import java.util.function.Supplier;
import java.util.stream.Collectors;
import lombok.AccessLevel;
import lombok.Getter;
+import lombok.Setter;
import org.awaitility.Awaitility;
import org.junit.Test;
import org.onap.policy.appc.Request;
@@ -55,6 +56,9 @@ public abstract class BaseTest {
private static final String APPC_RESTART_OP = "restart";
private static final String APPC_MODIFY_CONFIG_OP = "ModifyConfig";
+ private static final String SDNR_MODIFY_CONFIG_OP = "ModifyConfig";
+ private static final String SNDR_MODIFY_CONFIG_ANR_OP = "ModifyConfigANR";
+
/*
* Canonical Topic Names.
*/
@@ -135,6 +139,7 @@ public abstract class BaseTest {
// used to inject and wait for messages
@Getter(AccessLevel.PROTECTED)
+ @Setter(AccessLevel.PROTECTED)
protected static Topics topics;
// used to wait for messages on SINK topics
@@ -168,7 +173,7 @@ public abstract class BaseTest {
* Initializes {@link #topics} and {@link #controller}.
*/
public void init() {
- topics = topicMaker.get();
+ setTopics(topicMaker.get());
Map<String, SimpleLock> locks = getLockMap();
if (locks != null) {
@@ -348,7 +353,7 @@ public abstract class BaseTest {
*/
@Test
public void testVpciSunnyDayCompliant() {
- sdnrSunnyDay(VPCI_TOSCA_COMPLIANT_POLICY, VPCI_ONSET, VPCI_SDNR_SUCCESS, "ModifyConfig");
+ sdnrSunnyDay(VPCI_TOSCA_COMPLIANT_POLICY, VPCI_ONSET, VPCI_SDNR_SUCCESS, SDNR_MODIFY_CONFIG_OP);
}
// VSONH
@@ -358,7 +363,7 @@ public abstract class BaseTest {
*/
@Test
public void testVsonhSunnyDayCompliant() {
- sdnrSunnyDay(VSONH_TOSCA_COMPLIANT_POLICY, VSONH_ONSET, VSONH_SDNR_SUCCESS, "ModifyConfigANR");
+ sdnrSunnyDay(VSONH_TOSCA_COMPLIANT_POLICY, VSONH_ONSET, VSONH_SDNR_SUCCESS, SNDR_MODIFY_CONFIG_ANR_OP);
}
/**
diff --git a/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunnerTest2.java b/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunner2Test.java
index 1ed5b20bc..91e19ce79 100644
--- a/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunnerTest2.java
+++ b/controlloop/common/rules-test/src/test/java/org/onap/policy/controlloop/common/rules/test/NamedRunner2Test.java
@@ -32,7 +32,7 @@ import org.junit.runner.RunWith;
* executed.
*/
@RunWith(NamedRunner.class)
-public class NamedRunnerTest2 {
+public class NamedRunner2Test {
private static int testCount = 0;