Fix some sonar issues in drools-applications

Added coverage to:
- feature-controlloop-management

Fixed sonar issues, but didn't add coverage to:
- feature-controlloop-trans
- eventmanager
- guard

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

2 files changed, 61 insertions, 55 deletions

diff --git a/controlloop/common/feature-controlloop-trans/src/main/java/org/onap/policy/drools/apps/controlloop/feature/trans/CacheBasedControlLoopMetricsManager.java b/controlloop/common/feature-controlloop-trans/src/main/java/org/onap/policy/drools/apps/controlloop/feature/trans/CacheBasedControlLoopMetricsManager.java
index c5d6a32ac..672ff3fba 100644
--- a/controlloop/common/feature-controlloop-trans/src/main/java/org/onap/policy/drools/apps/controlloop/feature/trans/CacheBasedControlLoopMetricsManager.java
+++ b/controlloop/common/feature-controlloop-trans/src/main/java/org/onap/policy/drools/apps/controlloop/feature/trans/CacheBasedControlLoopMetricsManager.java
@@ -24,7 +24,6 @@ import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.cache.RemovalListener;
-import com.google.common.cache.RemovalNotification;
 
 import java.time.Instant;
 import java.time.ZonedDateTime;
@@ -47,6 +46,8 @@ import org.slf4j.LoggerFactory;
  */
 class CacheBasedControlLoopMetricsManager implements ControlLoopMetrics {
 
+    private static final String UNEXPECTED_NOTIFICATION_TYPE = "unexpected notification type {} in notification {}";
+
     private static final Logger logger = LoggerFactory.getLogger(CacheBasedControlLoopMetricsManager.class);
 
     private LoadingCache<UUID, VirtualControlLoopNotification> cache;
@@ -98,16 +99,12 @@ class CacheBasedControlLoopMetricsManager implements ControlLoopMetrics {
             }
         };
 
-        RemovalListener<UUID, VirtualControlLoopNotification> listener =
-                new RemovalListener<UUID, VirtualControlLoopNotification>() {
-            @Override
-            public void onRemoval(RemovalNotification<UUID, VirtualControlLoopNotification> notification) {
-                if (notification.wasEvicted()) {
-                    evicted(notification.getValue());
-                } else {
-                    logger.info("REMOVAL: {} because of {}", notification.getValue().getRequestId(),
-                                    notification.getCause().name());
-                }
+        RemovalListener<UUID, VirtualControlLoopNotification> listener = notification -> {
+            if (notification.wasEvicted()) {
+                evicted(notification.getValue());
+            } else {
+                logger.info("REMOVAL: {} because of {}", notification.getValue().getRequestId(),
+                                notification.getCause().name());
             }
         };
 
@@ -139,16 +136,11 @@ class CacheBasedControlLoopMetricsManager implements ControlLoopMetrics {
 
     @Override
     public void transactionEvent(PolicyController controller, VirtualControlLoopNotification notification) {
-        if (notification == null || notification.getRequestId() == null || notification.getNotification() == null) {
-            logger.warn("Invalid notification: {}", notification);
+        if (!isNotificationValid(notification)) {
             return;
         }
 
-        if (notification.getNotificationTime() == null) {
-            notification.setNotificationTime(ZonedDateTime.now());
-        }
-
-        notification.setFrom(notification.getFrom() + ":" + controller.getName());
+        setNotificationValues(controller, notification);
 
         switch (notification.getNotification()) {
             case REJECTED:
@@ -166,12 +158,29 @@ class CacheBasedControlLoopMetricsManager implements ControlLoopMetrics {
                 break;
             default:
                 /* unexpected */
-                logger.warn("unexpected notification type {} in notification {}",
-                        notification.getNotification().toString(), notification);
+                logger.warn(UNEXPECTED_NOTIFICATION_TYPE,
+                        notification.getNotification(), notification);
                 break;
         }
     }
 
+    private boolean isNotificationValid(VirtualControlLoopNotification notification) {
+        if (notification == null || notification.getRequestId() == null || notification.getNotification() == null) {
+            logger.warn("Invalid notification: {}", notification);
+            return false;
+        }
+
+        return true;
+    }
+
+    private void setNotificationValues(PolicyController controller, VirtualControlLoopNotification notification) {
+        if (notification.getNotificationTime() == null) {
+            notification.setNotificationTime(ZonedDateTime.now());
+        }
+
+        notification.setFrom(notification.getFrom() + ":" + controller.getName());
+    }
+
     @Override
     public VirtualControlLoopNotification getTransaction(UUID requestId) {
         return cache.getIfPresent(requestId);
@@ -260,13 +269,7 @@ class CacheBasedControlLoopMetricsManager implements ControlLoopMetrics {
                 trans.metric().resetTransaction();
                 break;
             case OPERATION:
-                trans.setStatusCode(true);
-                if (!operations.isEmpty()) {
-                    ControlLoopOperation operation = operations.get(operations.size() - 1);
-                    trans.setTargetEntity(operation.getTarget());
-                    trans.setTargetServiceName(operation.getActor());
-                }
-                trans.metric().resetTransaction();
+                metricOperation(trans, operations);
                 break;
             case OPERATION_SUCCESS:
                 trans.setStatusCode(true);
@@ -280,12 +283,22 @@ class CacheBasedControlLoopMetricsManager implements ControlLoopMetrics {
                 break;
             default:
                 /* unexpected */
-                logger.warn("unexpected notification type {} in notification {}",
-                        notification.getNotification().toString(), notification);
+                logger.warn(UNEXPECTED_NOTIFICATION_TYPE,
+                        notification.getNotification(), notification);
                 break;
         }
     }
 
+    private void metricOperation(MDCTransaction trans, List<ControlLoopOperation> operations) {
+        trans.setStatusCode(true);
+        if (!operations.isEmpty()) {
+            ControlLoopOperation operation = operations.get(operations.size() - 1);
+            trans.setTargetEntity(operation.getTarget());
+            trans.setTargetServiceName(operation.getActor());
+        }
+        trans.metric().resetTransaction();
+    }
+
     protected void operation(MDCTransaction trans, List<ControlLoopOperation> operations) {
         if (!operations.isEmpty()) {
             ControlLoopOperation operation = operations.get(operations.size() - 1);
@@ -340,7 +353,7 @@ class CacheBasedControlLoopMetricsManager implements ControlLoopMetrics {
                 break;
             default:
                 /* unexpected */
-                logger.warn("unexpected notification type {} in notification {}",
+                logger.warn(UNEXPECTED_NOTIFICATION_TYPE,
                         notification.getNotification(), notification);
                 break;
         }
diff --git a/controlloop/common/feature-controlloop-trans/src/test/java/org/onap/policy/drools/apps/controlloop/feature/trans/ControlLoopMetricsFeatureTest.java b/controlloop/common/feature-controlloop-trans/src/test/java/org/onap/policy/drools/apps/controlloop/feature/trans/ControlLoopMetricsFeatureTest.java
index 1f1e9de39..661803315 100644
--- a/controlloop/common/feature-controlloop-trans/src/test/java/org/onap/policy/drools/apps/controlloop/feature/trans/ControlLoopMetricsFeatureTest.java
+++ b/controlloop/common/feature-controlloop-trans/src/test/java/org/onap/policy/drools/apps/controlloop/feature/trans/ControlLoopMetricsFeatureTest.java
@@ -2,7 +2,7 @@
  * ============LICENSE_START=======================================================
  * ONAP
  * ================================================================================
- * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2018-2019 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.
@@ -20,6 +20,7 @@
 
 package org.onap.policy.drools.apps.controlloop.feature.trans;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -27,7 +28,6 @@ import static org.junit.Assert.assertTrue;
 
 import java.nio.file.Path;
 import java.util.UUID;
-
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -43,6 +43,7 @@ import org.onap.policy.drools.system.PolicyEngine;
  */
 public class ControlLoopMetricsFeatureTest {
 
+    private static final String POLICY_CL_MGT = "POLICY-CL-MGT";
     private static final Path configPath = SystemPersistence.manager.getConfigurationPath();
     private static PolicyController testController;
 
@@ -63,46 +64,42 @@ public class ControlLoopMetricsFeatureTest {
 
     @Test
     public void cacheDefaults() {
-        assertTrue(ControlLoopMetrics.manager.getCacheSize() == 3);
-        assertTrue(ControlLoopMetrics.manager.getTransactionTimeout() == 10);
-        assertTrue(ControlLoopMetrics.manager.getCacheOccupancy() == 0);
+        assertEquals(3, ControlLoopMetrics.manager.getCacheSize());
+        assertEquals(10, ControlLoopMetrics.manager.getTransactionTimeout());
+        assertEquals(0, ControlLoopMetrics.manager.getCacheOccupancy());
     }
 
     @Test
     public void invalidNotifications() {
         ControlLoopMetricsFeature feature = new ControlLoopMetricsFeature();
         VirtualControlLoopNotification notification = new VirtualControlLoopNotification();
-        feature.beforeDeliver(testController, CommInfrastructure.DMAAP, "POLICY-CL-MGT", notification);
+        feature.beforeDeliver(testController, CommInfrastructure.DMAAP, POLICY_CL_MGT, notification);
         this.cacheDefaults();
 
         UUID requestId = UUID.randomUUID();
         notification.setRequestId(requestId);
 
-        feature.beforeDeliver(testController, CommInfrastructure.DMAAP, "POLICY-CL-MGT", notification);
+        feature.beforeDeliver(testController, CommInfrastructure.DMAAP, POLICY_CL_MGT, notification);
         assertNull(ControlLoopMetrics.manager.getTransaction(requestId));
         this.cacheDefaults();
     }
 
     @Test
-    public void validActiveNotification() {
+    public void validActiveNotification() throws InterruptedException {
         ControlLoopMetricsFeature feature = new ControlLoopMetricsFeature();
         VirtualControlLoopNotification notification = new VirtualControlLoopNotification();
         UUID requestId = UUID.randomUUID();
         notification.setRequestId(requestId);
         notification.setNotification(ControlLoopNotificationType.ACTIVE);
 
-        feature.beforeDeliver(testController, CommInfrastructure.DMAAP, "POLICY-CL-MGT", notification);
+        feature.beforeDeliver(testController, CommInfrastructure.DMAAP, POLICY_CL_MGT, notification);
         assertNotNull(ControlLoopMetrics.manager.getTransaction(requestId));
         assertTrue(ControlLoopMetrics.manager.getTransaction(requestId).getFrom().contains(testController.getName()));
         assertNotNull(ControlLoopMetrics.manager.getTransaction(requestId).getNotificationTime());
         assertTrue(ControlLoopMetrics.manager.getCacheOccupancy() == 1);
 
         /* let the entries expire */
-        try {
-            Thread.sleep((ControlLoopMetrics.manager.getTransactionTimeout() + 5) * 1000L);
-        } catch (InterruptedException e) {
-            /* nothing to do */
-        }
+        Thread.sleep((ControlLoopMetrics.manager.getTransactionTimeout() + 1) * 1000L);
 
         assertNull(ControlLoopMetrics.manager.getTransaction(requestId));
         this.cacheDefaults();
@@ -111,7 +108,7 @@ public class ControlLoopMetricsFeatureTest {
     @Test
     public void reset() {
         VirtualControlLoopNotification notification = this.generateNotification();
-        new ControlLoopMetricsFeature().beforeDeliver(testController, CommInfrastructure.DMAAP, "POLICY-CL-MGT",
+        new ControlLoopMetricsFeature().beforeDeliver(testController, CommInfrastructure.DMAAP, POLICY_CL_MGT,
                 notification);
 
         assertNotNull(ControlLoopMetrics.manager.getTransaction(notification.getRequestId()));
@@ -135,19 +132,19 @@ public class ControlLoopMetricsFeatureTest {
     }
 
     @Test
-    public void eviction() {
+    public void eviction() throws InterruptedException {
         ControlLoopMetricsFeature feature = new ControlLoopMetricsFeature();
         for (int i = 0; i < ControlLoopMetrics.manager.getCacheSize(); i++) {
             VirtualControlLoopNotification notification = generateNotification();
-            feature.beforeDeliver(testController, CommInfrastructure.DMAAP, "POLICY-CL-MGT", notification);
+            feature.beforeDeliver(testController, CommInfrastructure.DMAAP, POLICY_CL_MGT, notification);
             assertNotNull(ControlLoopMetrics.manager.getTransaction(notification.getRequestId()));
         }
 
-        assertTrue(ControlLoopMetrics.manager.getCacheOccupancy() == ControlLoopMetrics.manager.getCacheOccupancy());
+        assertEquals(ControlLoopMetrics.manager.getCacheOccupancy(), ControlLoopMetrics.manager.getCacheOccupancy());
 
         VirtualControlLoopNotification overflowNotification = generateNotification();
-        feature.beforeDeliver(testController, CommInfrastructure.DMAAP, "POLICY-CL-MGT", overflowNotification);
-        assertTrue(ControlLoopMetrics.manager.getCacheOccupancy() == ControlLoopMetrics.manager.getCacheOccupancy());
+        feature.beforeDeliver(testController, CommInfrastructure.DMAAP, POLICY_CL_MGT, overflowNotification);
+        assertEquals(ControlLoopMetrics.manager.getCacheOccupancy(), ControlLoopMetrics.manager.getCacheOccupancy());
         assertNotNull(ControlLoopMetrics.manager.getTransaction(overflowNotification.getRequestId()));
         assertTrue(ControlLoopMetrics.manager.getTransactionIds().size() == ControlLoopMetrics.manager.getCacheSize());
         assertTrue(ControlLoopMetrics.manager.getCacheOccupancy() == ControlLoopMetrics.manager.getCacheSize());
@@ -155,11 +152,7 @@ public class ControlLoopMetricsFeatureTest {
         assertFalse(ControlLoopMetrics.manager.getTransactions().isEmpty());
 
         /* let the entries expire */
-        try {
-            Thread.sleep((ControlLoopMetrics.manager.getTransactionTimeout() + 5) * 1000L);
-        } catch (InterruptedException e) {
-            /* nothing to do */
-        }
+        Thread.sleep((ControlLoopMetrics.manager.getTransactionTimeout() + 1) * 1000L);
 
         ControlLoopMetrics.manager.refresh();
         assertTrue(ControlLoopMetrics.manager.getTransactionIds().size() == ControlLoopMetrics.manager