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>
diff --git a/common-logging/pom.xml b/common-logging/pom.xml
index 1fef17a..a87d31a 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 4b5c57a..ddcf7f8 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 @@
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 @@
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 f1b25d7..c32cf0b 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.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 @@
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 @@
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 dc74044..d21af4c 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 @@
// 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 030363d..f5e84ac 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 @@
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 8802d17..fc0995b 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 @@
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 38759bc..41539f2 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 @@
* 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 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 6af3632..db4eb78 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_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 @@
@Test
public void testInitNullProperties() {
- PolicyLogger.init(null);
+ assertThatCode(() -> PolicyLogger.init(null)).doesNotThrowAnyException();
}
@Test
@@ -630,7 +631,7 @@
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 a74dd94..ebc4b21 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 @@
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 99c343c..d174aaf 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 @@
@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 92df029..6804fd0 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 @@
@Test
public void testSystemOutLoggerClassOfQ() {
- new SystemOutLogger(SystemOutLoggerTest.class);
+ assertThatCode(() -> new SystemOutLogger(SystemOutLoggerTest.class)).doesNotThrowAnyException();
}
@Test