Address sonar issues in ONAP-logging

Addressed the following sonar issues in ONAP-logging:
- use Map instead of ConcurrentHashMap
- remove unusued fields
- use Map.computeIfAbsent() instead of get()/put() pair
- readObject is unsafe
- use try-with-resources
- junit should assert something

Also removed some unused imports.

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

11 files changed, 43 insertions, 37 deletions

diff --git a/common-logging/pom.xml b/common-logging/pom.xml
index 1fef17a1..a87d31a1 100644
--- a/common-logging/pom.xml
+++ b/common-logging/pom.xml
@@ -67,6 +67,11 @@
             <artifactId>commons-lang3</artifactId>
         </dependency>
         <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
             <groupId>org.powermock</groupId>
             <artifactId>powermock-api-mockito2</artifactId>
             <scope>test</scope>
diff --git a/common-logging/src/main/java/org/onap/policy/common/logging/eelf/EventTrackInfoHandler.java b/common-logging/src/main/java/org/onap/policy/common/logging/eelf/EventTrackInfoHandler.java
index 4b5c57a8..ddcf7f8e 100644
--- a/common-logging/src/main/java/org/onap/policy/common/logging/eelf/EventTrackInfoHandler.java
+++ b/common-logging/src/main/java/org/onap/policy/common/logging/eelf/EventTrackInfoHandler.java
@@ -23,8 +23,8 @@ package org.onap.policy.common.logging.eelf;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Map;
 import java.util.TimerTask;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
 /**
@@ -63,7 +63,7 @@ public class EventTrackInfoHandler extends TimerTask {
 
         ArrayList<String> expiredEvents = null;
 
-        for (ConcurrentHashMap.Entry<String, EventData> entry : eventInfo.entrySet()) {
+        for (Map.Entry<String, EventData> entry : eventInfo.entrySet()) {
             EventData event = entry.getValue();
             startTime = event.getStartTime();
             ns = Duration.between(startTime, Instant.now()).getSeconds();
diff --git a/common-logging/src/main/java/org/onap/policy/common/logging/eelf/PolicyLogger.java b/common-logging/src/main/java/org/onap/policy/common/logging/eelf/PolicyLogger.java
index f1b25d71..c32cf0bb 100644
--- a/common-logging/src/main/java/org/onap/policy/common/logging/eelf/PolicyLogger.java
+++ b/common-logging/src/main/java/org/onap/policy/common/logging/eelf/PolicyLogger.java
@@ -57,7 +57,6 @@ import java.util.Date;
 import java.util.Iterator;
 import java.util.Properties;
 import java.util.Timer;
-import java.util.TimerTask;
 import java.util.UUID;
 import java.util.concurrent.ConcurrentMap;
 import java.util.function.Consumer;
@@ -86,7 +85,6 @@ public class PolicyLogger {
     private static String hostAddress = null;
     private static String component = null;
 
-    private static TimerTask ttrcker = null;
     private static boolean isEventTrackerRunning = false;
     private static Timer timer = null;
 
@@ -1186,7 +1184,7 @@ public class PolicyLogger {
     private static void startCleanUp() {
 
         if (!isEventTrackerRunning) {
-            ttrcker = new EventTrackInfoHandler();
+            EventTrackInfoHandler ttrcker = new EventTrackInfoHandler();
             timer = new Timer(true);
             timer.scheduleAtFixedRate(ttrcker, timerDelayTime, checkInterval);
             debugLogger.info("EventTrackInfoHandler begins! : " + new Date());
diff --git a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/DisplayUtils.java b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/DisplayUtils.java
index dc740440..d21af4c1 100644
--- a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/DisplayUtils.java
+++ b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/DisplayUtils.java
@@ -31,11 +31,16 @@ public class DisplayUtils {
         // do nothing
     }
 
+    /*
+     * As the comment above says, these purposely write to System.out rather than a
+     * logger, thus sonar is disabled.
+     */
+
     public static void displayMessage(Object message) {
-        System.out.println(message);
+        System.out.println(message);    // NOSONAR
     }
 
     public static void displayErrorMessage(Object msg) {
-        System.err.println(msg);
+        System.err.println(msg);        // NOSONAR
     }
 }
diff --git a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/FlexLogger.java b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/FlexLogger.java
index 030363dc..f5e84ac3 100644
--- a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/FlexLogger.java
+++ b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/FlexLogger.java
@@ -209,21 +209,13 @@ public class FlexLogger extends SecurityManager {
             className = new FlexLogger().getClassName();
         }
 
-        if (!eelfLoggerMap.containsKey(className)) {
-            logger = new EelfLogger(clazz, isNewTransaction);
-            eelfLoggerMap.put(className, logger);
-        } else {
-            logger = eelfLoggerMap.get(className);
-            if (logger == null) {
-                logger = new EelfLogger(clazz, isNewTransaction);
-                eelfLoggerMap.put(className, logger);
-            }
-            // installl already created but it is new transaction
-            if (isNewTransaction) {
-                String transId = PolicyLogger.postMdcInfoForEvent(null);
-                logger.setTransId(transId);
-            }
+        logger = eelfLoggerMap.computeIfAbsent(className, key -> new EelfLogger(clazz, isNewTransaction));
+
+        if (isNewTransaction) {
+            String transId = PolicyLogger.postMdcInfoForEvent(null);
+            logger.setTransId(transId);
         }
+
         displayMessage("eelfLoggerMap size : " + eelfLoggerMap.size() + " class name: " + className);
         return logger;
     }
diff --git a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/Logger4J.java b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/Logger4J.java
index 8802d17e..fc0995ba 100644
--- a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/Logger4J.java
+++ b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/Logger4J.java
@@ -496,17 +496,17 @@ public class Logger4J implements org.onap.policy.common.logging.flexlogger.Logge
 
     private void writeObject(ObjectOutputStream out) throws IOException {
         // write out 'methodName', 'className', 'transId' strings
-        out.writeObject(methodName);
-        out.writeObject(className);
-        out.writeObject(transId);
+        out.writeUTF(methodName);
+        out.writeUTF(className);
+        out.writeUTF(transId);
     }
 
     private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
 
         // read in 'methodName', 'className', 'transId' strings
-        methodName = (String) (in.readObject());
-        className = (String) (in.readObject());
-        transId = (String) (in.readObject());
+        methodName = in.readUTF();
+        className = in.readUTF();
+        transId = in.readUTF();
 
         // look up associated logger
         log = Logger.getLogger(className);
diff --git a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/PropertyUtil.java b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/PropertyUtil.java
index 38759bc2..41539f20 100644
--- a/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/PropertyUtil.java
+++ b/common-logging/src/main/java/org/onap/policy/common/logging/flexlogger/PropertyUtil.java
@@ -45,6 +45,10 @@ public class PropertyUtil {
          * This may be overridden by junit tests.
          */
         private static Timer timer = new Timer("PropertyUtil-Timer", true);
+
+        private LazyHolder() {
+            super();
+        }
     }
 
     // this table maps canonical file into a 'ListenerRegistration' instance
@@ -60,17 +64,13 @@ public class PropertyUtil {
      */
     public static Properties getProperties(File file) throws IOException {
         // create an InputStream (may throw a FileNotFoundException)
-        FileInputStream fis = new FileInputStream(file);
-        try {
+        try (FileInputStream fis = new FileInputStream(file)) {
             // create the properties instance
             Properties rval = new Properties();
 
             // load properties (may throw an IOException)
             rval.load(fis);
             return rval;
-        } finally {
-            // close input stream
-            fis.close();
         }
     }
 
diff --git a/common-logging/src/test/java/org/onap/policy/common/logging/eelf/PolicyLoggerTest.java b/common-logging/src/test/java/org/onap/policy/common/logging/eelf/PolicyLoggerTest.java
index 6af3632a..db4eb786 100644
--- a/common-logging/src/test/java/org/onap/policy/common/logging/eelf/PolicyLoggerTest.java
+++ b/common-logging/src/test/java/org/onap/policy/common/logging/eelf/PolicyLoggerTest.java
@@ -30,6 +30,7 @@ import static com.att.eelf.configuration.Configuration.MDC_SERVER_FQDN;
 import static com.att.eelf.configuration.Configuration.MDC_SERVER_IP_ADDRESS;
 import static com.att.eelf.configuration.Configuration.MDC_SERVICE_INSTANCE_ID;
 import static com.att.eelf.configuration.Configuration.MDC_SERVICE_NAME;
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -613,7 +614,7 @@ public class PolicyLoggerTest {
 
     @Test
     public void testInitNullProperties() {
-        PolicyLogger.init(null);
+        assertThatCode(() -> PolicyLogger.init(null)).doesNotThrowAnyException();
     }
 
     @Test
@@ -630,7 +631,7 @@ public class PolicyLoggerTest {
         properties.setProperty("stop.check.point", "0");
         properties.setProperty("logger.property", "LOG4J");
 
-        PolicyLogger.init(properties);
+        assertThatCode(() -> PolicyLogger.init(properties)).doesNotThrowAnyException();
     }
 
     @Test
diff --git a/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/FlexLoggerTest.java b/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/FlexLoggerTest.java
index a74dd94d..ebc4b212 100644
--- a/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/FlexLoggerTest.java
+++ b/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/FlexLoggerTest.java
@@ -21,6 +21,7 @@
 
 package org.onap.policy.common.logging.flexlogger;
 
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertSame;
 
@@ -131,7 +132,9 @@ public class FlexLoggerTest {
         changedKeys.add("error.level");
         changedKeys.add("audit.level");
         PropertiesCallBack propertiesCallBack = new PropertiesCallBack("name");
-        propertiesCallBack.propertiesChanged(PropertyUtil.getProperties("config/policyLogger.properties"), changedKeys);
+        assertThatCode(() -> propertiesCallBack
+                        .propertiesChanged(PropertyUtil.getProperties("config/policyLogger.properties"), changedKeys))
+                                        .doesNotThrowAnyException();
     }
 
 }
diff --git a/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/Logger4JTest.java b/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/Logger4JTest.java
index 99c343c0..d174aafc 100644
--- a/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/Logger4JTest.java
+++ b/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/Logger4JTest.java
@@ -21,6 +21,7 @@
 
 package org.onap.policy.common.logging.flexlogger;
 
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
@@ -41,7 +42,7 @@ public class Logger4JTest {
 
     @Test
     public void testLogger4JClassOfQ() {
-        new Logger4J(this.getClass());
+        assertThatCode(() -> new Logger4J(this.getClass())).doesNotThrowAnyException();
     }
 
     @Test
diff --git a/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/SystemOutLoggerTest.java b/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/SystemOutLoggerTest.java
index 92df0297..6804fd0c 100644
--- a/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/SystemOutLoggerTest.java
+++ b/common-logging/src/test/java/org/onap/policy/common/logging/flexlogger/SystemOutLoggerTest.java
@@ -21,6 +21,7 @@
 
 package org.onap.policy.common.logging.flexlogger;
 
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -41,7 +42,7 @@ public class SystemOutLoggerTest {
 
     @Test
     public void testSystemOutLoggerClassOfQ() {
-        new SystemOutLogger(SystemOutLoggerTest.class);
+        assertThatCode(() -> new SystemOutLogger(SystemOutLoggerTest.class)).doesNotThrowAnyException();
     }
 
     @Test