Address more sonars in drools-pdp 20/111720/1
authorJim Hahn <jrh3@att.com>
Tue, 25 Aug 2020 21:05:37 +0000 (17:05 -0400)
committerJim Hahn <jrh3@att.com>
Tue, 25 Aug 2020 22:47:23 +0000 (18:47 -0400)
Addressed the following sonars:
- either log or rethrow
- call "remove()" for thread-local-storage
- use assertEquals
- only one method call in exception test
- swap arguments in assertEquals
- add assertion to assertThatThrownBy()
- explain @Ignore

Also addressed eclipse warnings:
- unused fields and methods

Issue-ID: POLICY-2616
Change-Id: I6590c0d2b103885bc933014d48bf5fd92401cd80
Signed-off-by: Jim Hahn <jrh3@att.com>
17 files changed:
feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateUnsupportedTest.java
feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/DmaapManagerTest.java
feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/EndToEndFeatureTest.java
feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/SerializerTest.java
feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/state/ProcessingStateTest.java
feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/PersistenceFeature.java
feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/PersistenceFeatureTest.java
feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java
policy-core/src/main/java/org/onap/policy/drools/core/PolicySession.java
policy-core/src/test/java/org/onap/policy/drools/core/PolicySessionTest.java
policy-domains/src/test/java/org/onap/policy/drools/domain/models/DomainPolicyTypesTest.java
policy-management/src/main/java/org/onap/policy/drools/system/Main.java
policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/ControllerConfigurationTest.java
policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/DroolsConfigurationTest.java
policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/PdpdConfigurationTest.java
policy-utils/src/test/java/org/onap/policy/drools/policies/DomainMakerTest.java
policy-utils/src/test/java/org/onap/policy/drools/utils/logging/MdcTransactionTest.java

index 1b7c62a..400aa16 100644 (file)
@@ -90,19 +90,22 @@ public abstract class LifecycleStateUnsupportedTest {
 
     @Test
     public void update() {
-        assertThatThrownBy(() -> state.update(new PdpUpdate()))
+        PdpUpdate update = new PdpUpdate();
+        assertThatThrownBy(() -> state.update(update))
             .isInstanceOf(UnsupportedOperationException.class);
     }
 
     @Test
     public void stateChange() {
-        assertThatThrownBy(() -> state.stateChange(new PdpStateChange()))
+        PdpStateChange change = new PdpStateChange();
+        assertThatThrownBy(() -> state.stateChange(change))
             .isInstanceOf(UnsupportedOperationException.class);
     }
 
     @Test
     public void changeState() {
-        assertThatThrownBy(() -> state.transitionToState(new LifecycleStateActive(new LifecycleFsm())))
+        LifecycleStateActive active = new LifecycleStateActive(new LifecycleFsm());
+        assertThatThrownBy(() -> state.transitionToState(active))
             .isInstanceOf(UnsupportedOperationException.class);
     }
 }
index d5b397a..7f73a70 100644 (file)
@@ -290,7 +290,7 @@ public class DmaapManagerTest {
     @Test
     public void testPublish() throws PoolingFeatureException {
         // cannot publish before starting
-        assertThatThrownBy(() -> mgr.publish(MSG)).as("publish,pre");
+        assertThatThrownBy(() -> mgr.publish(MSG)).as("publish,pre").isInstanceOf(PoolingFeatureException.class);
 
         mgr.startPublisher();
 
@@ -306,7 +306,7 @@ public class DmaapManagerTest {
 
         // stop and verify we can no longer publish
         mgr.stopPublisher(0);
-        assertThatThrownBy(() -> mgr.publish(MSG)).as("publish,stopped");
+        assertThatThrownBy(() -> mgr.publish(MSG)).as("publish,stopped").isInstanceOf(PoolingFeatureException.class);
     }
 
     @Test(expected = PoolingFeatureException.class)
index bee25ff..08c7ebe 100644 (file)
@@ -2,7 +2,7 @@
  * ============LICENSE_START=======================================================
  * ONAP
  * ================================================================================
- * Copyright (C) 2018-2019 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2018-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.
@@ -178,21 +178,33 @@ public class EndToEndFeatureTest {
         }
     }
 
+    /*
+     * This test should only be run manually, after configuring all of the fields,
+     * thus it is ignored.
+     */
     @Ignore
     @Test
-    public void test_SingleHost() throws Exception {
+    public void test_SingleHost() throws Exception {    // NOSONAR
         run(70, 1);
     }
 
+    /*
+     * This test should only be run manually, after configuring all of the fields,
+     * thus it is ignored.
+     */
     @Ignore
     @Test
-    public void test_TwoHosts() throws Exception {
+    public void test_TwoHosts() throws Exception {      // NOSONAR
         run(200, 2);
     }
 
+    /*
+     * This test should only be run manually, after configuring all of the fields,
+     * thus it is ignored.
+     */
     @Ignore
     @Test
-    public void test_ThreeHosts() throws Exception {
+    public void test_ThreeHosts() throws Exception {    // NOSONAR
         run(200, 3);
     }
 
index 09e8385..662e0a7 100644 (file)
@@ -97,7 +97,8 @@ public class SerializerTest {
         assertEquals(msg.getChannel(), decoded.getChannel());
 
         // invalid subclass when encoding
-        assertThatThrownBy(() -> ser.encodeMsg(new Message() {})).isInstanceOf(JsonParseException.class)
+        Message msg2 = new Message() {};
+        assertThatThrownBy(() -> ser.encodeMsg(msg2)).isInstanceOf(JsonParseException.class)
                         .hasMessageContaining("cannot serialize");
 
         // missing type when decoding
index 82346f5..7dc7b2f 100644 (file)
@@ -2,7 +2,7 @@
  * ============LICENSE_START=======================================================
  * ONAP
  * ================================================================================
- * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2018, 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.
@@ -269,7 +269,7 @@ public class ProcessingStateTest extends SupportBasicStateTester {
 
         String[] arr = captureHostArray();
 
-        assertNotSame(arr, HOST_ARR3);
+        assertNotSame(HOST_ARR3, arr);
         assertEquals(Arrays.asList(HOST_ARR3), Arrays.asList(arr));
     }
 
index ed42725..efe2a29 100644 (file)
@@ -767,6 +767,7 @@ public class PersistenceFeature implements PolicySessionFeatureApi, PolicyEngine
                 }
             }
 
+            session.removePolicySession();
             logger.info("PersistentThreadModel completed");
         }
     }
index 25f805d..ec26198 100644 (file)
@@ -1098,15 +1098,9 @@ public class PersistenceFeatureTest {
         // return adjunct on next call
         when(polcont.getAdjunct(any())).thenReturn(adjcap.getValue());
 
-        try {
-            doThrow(new IllegalArgumentException(EXPECTED)).when(emf).close();
-
-            feat.destroyKieSession(polsess);
-            fail(MISSING_EXCEPTION);
-
-        } catch (IllegalArgumentException ex) {
-            logger.trace(EXPECTED, ex);
-        }
+        IllegalArgumentException exception = new IllegalArgumentException(EXPECTED);
+        doThrow(exception).when(emf).close();
+        assertThatCode(() -> feat.destroyKieSession(polsess)).isEqualTo(exception);
 
         verify(bds, times(2)).close();
     }
@@ -1126,15 +1120,10 @@ public class PersistenceFeatureTest {
         // return adjunct on next call
         when(polcont.getAdjunct(any())).thenReturn(adjcap.getValue());
 
-        try {
-            doThrow(new SQLException(EXPECTED)).when(bds).close();
-
-            feat.destroyKieSession(polsess);
-            fail(MISSING_EXCEPTION);
-
-        } catch (PersistenceFeatureException ex) {
-            logger.trace(EXPECTED, ex);
-        }
+        SQLException cause = new SQLException(EXPECTED);
+        doThrow(cause).when(bds).close();
+        assertThatCode(() -> feat.destroyKieSession(polsess)).isInstanceOf(PersistenceFeatureException.class)
+                        .hasCause(cause);
     }
 
     /**
index 347e8c9..33bfaed 100644 (file)
@@ -20,6 +20,7 @@
 
 package org.onap.policy.drools.statemanagement.test;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
@@ -143,8 +144,8 @@ public class StateManagementTest {
         logger.debug("avail = {}", avail);
         logger.debug("standby = {}", standby);
 
-        assertTrue("Admin state not unlocked after initialization", admin.equals(StateManagement.UNLOCKED));
-        assertTrue("Operational state not enabled after initialization", oper.equals(StateManagement.ENABLED));
+        assertEquals("Admin state not unlocked after initialization", StateManagement.UNLOCKED, admin);
+        assertEquals("Operational state not enabled after initialization", StateManagement.ENABLED, oper);
 
         try {
             stateManagementFeature.disableFailed();
@@ -164,8 +165,8 @@ public class StateManagementTest {
         logger.debug("avail = {}", avail);
         logger.debug("standby = {}", standby);
 
-        assertTrue("Operational state not disabled after disableFailed()", oper.equals(StateManagement.DISABLED));
-        assertTrue("Availability status not failed after disableFailed()", avail.equals(StateManagement.FAILED));
+        assertEquals("Operational state not disabled after disableFailed()", StateManagement.DISABLED, oper);
+        assertEquals("Availability status not failed after disableFailed()", StateManagement.FAILED, avail);
 
 
         try {
@@ -185,7 +186,7 @@ public class StateManagementTest {
         logger.debug("avail = {}", avail);
         logger.debug("standby = {}", standby);
 
-        assertTrue("Standby status not coldstandby after promote()", standby.equals(StateManagement.COLD_STANDBY));
+        assertEquals("Standby status not coldstandby after promote()", StateManagement.COLD_STANDBY, standby);
 
         /* *************Repository Audit Test. ************* */
         logger.debug("\n\ntestStateManagementOperation: Repository Audit\n\n");
index 49ff7bb..389d5b3 100644 (file)
@@ -189,6 +189,16 @@ public class PolicySession
         policySess.set(this);
     }
 
+    /**
+     * Unset this 'PolicySession' instance as the one associated with the
+     * currently-running thread.
+     */
+    public void removePolicySession() {
+        if (policySess.get() == this) {
+            policySess.remove();
+        }
+    }
+
     /**
      * Get current session.
      *
@@ -498,6 +508,8 @@ public class PolicySession
                     logger.error("startThread error in kieSession1.fireUntilHalt", e);
                 }
             }
+
+            session.removePolicySession();
             logger.info("fireUntilHalt() returned");
         }
     }
index 63c7160..3b88219 100644 (file)
@@ -102,7 +102,7 @@ public class PolicySessionTest {
     }
 
     @Test
-    public void testSetPolicySession_testGetCurrentSession() {
+    public void testSetPolicySession_testGetCurrentSession_testRemovePolicySession() {
         PolicySession sess2 = new PolicySession(MY_NAME + "-b", container, kie);
 
         session.setPolicySession();
@@ -110,6 +110,14 @@ public class PolicySessionTest {
 
         sess2.setPolicySession();
         assertEquals(sess2, PolicySession.getCurrentSession());
+
+        // remove a different session - should be unchanged
+        session.removePolicySession();
+        assertEquals(sess2, PolicySession.getCurrentSession());
+
+        // remove the session
+        sess2.removePolicySession();
+        assertNull(PolicySession.getCurrentSession());
     }
 
     @Test
index 77dfed8..f737766 100644 (file)
@@ -47,14 +47,8 @@ import org.onap.policy.models.tosca.authorative.concepts.ToscaServiceTemplate;
 public class DomainPolicyTypesTest {
 
     // Policy Types
-    private static final String OPERATIONAL_DROOLS_POLICY_TYPE = "onap.policies.controlloop.operational.common.Drools";
     private static final String NATIVE_DROOLS_POLICY_TYPE = "onap.policies.native.drools.Artifact";
 
-    // Operational vCPE Policy
-    private static final String OP_POLICY_NAME_VCPE = "operational.restart";
-    private static final String VCPE_OPERATIONAL_DROOLS_POLICY_JSON =
-            "policies/vCPE.policy.operational.input.tosca.json";
-
     // Native Drools Policy
     private static final String EXAMPLE_NATIVE_DROOLS_POLICY_NAME = "example";
     private static final String EXAMPLE_NATIVE_DROOLS_POLICY_JSON =
@@ -184,8 +178,4 @@ public class DomainPolicyTypesTest {
         ToscaServiceTemplate serviceTemplate = new StandardCoder().decode(policyJson, ToscaServiceTemplate.class);
         return serviceTemplate.getToscaTopologyTemplate().getPolicies().get(0).get(policyName);
     }
-
-    private String getExamplesPolicyString(String resourcePath, String policyName) throws CoderException {
-        return nonValCoder.encode(getExamplesPolicy(resourcePath, policyName));
-    }
-}
\ No newline at end of file
+}
index 4f0dc39..9e6d043 100644 (file)
@@ -157,10 +157,8 @@ public class Main {
                 .flush();
             logger.warn(
                     LoggerUtil.TRANSACTION_LOG_MARKER,
-                    "Main: cannot start {} (bad state) because of {}",
-                    PolicyEngineConstants.getManager(),
-                    e.getMessage(),
-                    e);
+                    "Main: cannot start {} (bad state)",
+                    PolicyEngineConstants.getManager());
             throw new PolicyDroolsPdpRuntimeException(
                     String.format(MessageConstants.START_FAILURE_MSG, MessageConstants.POLICY_DROOLS_PDP), e);
         } catch (final Exception e) {
@@ -171,10 +169,8 @@ public class Main {
                 .flush();
             logger.warn(
                     LoggerUtil.TRANSACTION_LOG_MARKER,
-                    "Main: cannot start {} because of {}",
-                    PolicyEngineConstants.getManager(),
-                    e.getMessage(),
-                    e);
+                    "Main: cannot start {}",
+                    PolicyEngineConstants.getManager());
             throw new PolicyDroolsPdpRuntimeException(
                     String.format(MessageConstants.START_FAILURE_MSG, MessageConstants.POLICY_DROOLS_PDP), e);
         }
@@ -209,10 +205,8 @@ public class Main {
                     .flush();
                 logger.error(
                         LoggerUtil.TRANSACTION_LOG_MARKER,
-                        "Main: cannot instantiate policy-controller {} because of {}",
-                        controllerName,
-                        e.getMessage(),
-                        e);
+                        "Main: cannot instantiate policy-controller {}",
+                        controllerName);
                 throw new PolicyDroolsPdpRuntimeException(
                         String.format(MessageConstants.START_FAILURE_MSG, MessageConstants.POLICY_DROOLS_PDP), e);
             } catch (final LinkageError e) {
@@ -223,10 +217,8 @@ public class Main {
                     .flush();
                 logger.warn(
                         LoggerUtil.TRANSACTION_LOG_MARKER,
-                        "Main: cannot instantiate policy-controller {} (linkage) because of {}",
-                        controllerName,
-                        e.getMessage(),
-                        e);
+                        "Main: cannot instantiate policy-controller {} (linkage)",
+                        controllerName);
                 throw new PolicyDroolsPdpRuntimeException(
                         String.format(MessageConstants.START_FAILURE_MSG, MessageConstants.POLICY_DROOLS_PDP), e);
             }
index 3741e71..3560671 100644 (file)
@@ -66,7 +66,7 @@ public class ControllerConfigurationTest {
 
         ControllerConfiguration controllerConfig = new ControllerConfiguration(NAME, OPERATION, DROOLS_CONFIG);
 
-        assertEquals(controllerConfig, controllerConfig);
+        assertEquals(controllerConfig, (Object) controllerConfig);
         assertNotEquals(controllerConfig, new Object());
 
         ControllerConfiguration controllerConfig2 = new ControllerConfiguration();
index 92726f8..a18a20b 100644 (file)
@@ -56,7 +56,7 @@ public class DroolsConfigurationTest {
         additionalProperties.put(ADDITIONAL_PROPERTY_KEY, ADDITIONAL_PROPERTY_VALUE);
 
         final DroolsConfiguration droolsConfig = new DroolsConfiguration(ARTIFACT, GROUPID, VERSION);
-        assertEquals(droolsConfig, droolsConfig);
+        assertEquals(droolsConfig, (Object) droolsConfig);
 
         droolsConfig.set(ARTIFACT_ID_STRING, "foobar");
         assertEquals("foobar", droolsConfig.get(ARTIFACT_ID_STRING));
index 352d8ce..b02090b 100644 (file)
@@ -74,7 +74,7 @@ public class PdpdConfigurationTest {
         drools.set("version", VERSION);
         drools.set(PROPERTY1, VALUE1);
 
-        assertEquals(drools, drools);
+        assertEquals(drools, (Object) drools);
         assertNotEquals(drools, new Object());
 
         logger.info("Drools HashCode {}", drools.hashCode());
@@ -131,7 +131,7 @@ public class PdpdConfigurationTest {
         controller.set("drools", drools);
         controller.set(PROPERTY1, VALUE1);
 
-        assertEquals(controller, controller);
+        assertEquals(controller, (Object) controller);
         assertNotEquals(controller, new Object());
 
         logger.info("Controller HashCode {}", controller.hashCode());
@@ -194,7 +194,7 @@ public class PdpdConfigurationTest {
         config.set("controllers", controllers);
         config.set(PROPERTY1, VALUE1);
 
-        assertEquals(config, config);
+        assertEquals(config, (Object) config);
         assertNotEquals(config, new Object());
 
         logger.info("Config HashCode {}", config.hashCode());
index 47d6c2d..5bc767c 100644 (file)
@@ -108,7 +108,8 @@ public class DomainMakerTest {
         assertDomainPolicy(domainAPolicy);
 
         domainAPolicy.getProperties().getNested().setNested1("");
-        assertThatThrownBy(() -> domainMaker.conformance(policy1.getTypeIdentifier(), domainAPolicy))
+        ToscaPolicyTypeIdentifier ident1 = policy1.getTypeIdentifier();
+        assertThatThrownBy(() -> domainMaker.conformance(ident1, domainAPolicy))
                 .isInstanceOf(ValidationFailedException.class)
                 .hasMessageContaining("Pattern ^(.+)$ is not contained in text");
     }
@@ -142,8 +143,9 @@ public class DomainMakerTest {
 
     @Test
     public void testConvertToSchema() {
+        ToscaPolicyType type = new ToscaPolicyType();
         assertThatThrownBy(() -> domainMaker
-            .convertToSchema(new ToscaPolicyType()))
+            .convertToSchema(type))
             .isInstanceOf(UnsupportedOperationException.class);
     }
 
index dbf6a67..82a2b2e 100644 (file)
@@ -164,7 +164,6 @@ public class MdcTransactionTest {
         assertEquals(trans.getInvocationId(), MDC.get(MdcTransactionConstants.INVOCATION_ID));
         assertEquals(trans.timestamp(trans.getStartTime()), MDC.get(MdcTransactionConstants.BEGIN_TIMESTAMP));
         assertEquals(trans.timestamp(trans.getEndTime()), MDC.get(MdcTransactionConstants.END_TIMESTAMP));
-        assertNotEquals(trans.getElapsedTime(), MDC.get(MdcTransactionConstants.ELAPSED_TIME));
         assertEquals(String.valueOf(Duration.between(trans.getStartTime(), trans.getEndTime()).toMillis()),
             MDC.get(MdcTransactionConstants.ELAPSED_TIME));
         assertEquals(trans.getServiceInstanceId(), MDC.get(MdcTransactionConstants.SERVICE_INSTANCE_ID));