From 9f9131575d2e2b1002a3e108f7793a97fa7652ab Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Mon, 6 Apr 2020 12:17:11 -0400 Subject: 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 Change-Id: I3480a55da4d0e771f8083c97770a6c9707d871f7 Signed-off-by: Jim Hahn --- common-logging/pom.xml | 5 +++++ .../common/logging/eelf/EventTrackInfoHandler.java | 4 ++-- .../policy/common/logging/eelf/PolicyLogger.java | 4 +--- .../common/logging/flexlogger/DisplayUtils.java | 9 +++++++-- .../policy/common/logging/flexlogger/FlexLogger.java | 20 ++++++-------------- .../policy/common/logging/flexlogger/Logger4J.java | 12 ++++++------ .../common/logging/flexlogger/PropertyUtil.java | 10 +++++----- .../policy/common/logging/eelf/PolicyLoggerTest.java | 5 +++-- .../common/logging/flexlogger/FlexLoggerTest.java | 5 ++++- .../common/logging/flexlogger/Logger4JTest.java | 3 ++- .../logging/flexlogger/SystemOutLoggerTest.java | 3 ++- 11 files changed, 43 insertions(+), 37 deletions(-) (limited to 'common-logging') 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 @@ -66,6 +66,11 @@ org.apache.commons commons-lang3 + + org.assertj + assertj-core + test + org.powermock powermock-api-mockito2 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 expiredEvents = null; - for (ConcurrentHashMap.Entry entry : eventInfo.entrySet()) { + for (Map.Entry 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 -- cgit 1.2.3-korg