From d4e2772ba361bc23a2c858e7a70207ec451a77e1 Mon Sep 17 00:00:00 2001 From: kezhenxu94 Date: Wed, 30 Jan 2019 10:30:44 +0800 Subject: [PATCH 1/7] polishing LoggerFactory --- .../dubbo/common/logger/LoggerFactory.java | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java index ec98755000bc..e4f715fd3ee4 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java @@ -20,11 +20,14 @@ import org.apache.dubbo.common.logger.jcl.JclLoggerAdapter; import org.apache.dubbo.common.logger.jdk.JdkLoggerAdapter; import org.apache.dubbo.common.logger.log4j.Log4jLoggerAdapter; +import org.apache.dubbo.common.logger.log4j2.Log4j2Logger; import org.apache.dubbo.common.logger.log4j2.Log4j2LoggerAdapter; import org.apache.dubbo.common.logger.slf4j.Slf4jLoggerAdapter; import org.apache.dubbo.common.logger.support.FailsafeLogger; import java.io.File; +import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -40,34 +43,37 @@ public class LoggerFactory { // search common-used logging frameworks static { String logger = System.getProperty("dubbo.application.logger"); - if ("slf4j".equals(logger)) { - setLoggerAdapter(new Slf4jLoggerAdapter()); - } else if ("jcl".equals(logger)) { - setLoggerAdapter(new JclLoggerAdapter()); - } else if ("log4j".equals(logger)) { - setLoggerAdapter(new Log4jLoggerAdapter()); - } else if ("jdk".equals(logger)) { - setLoggerAdapter(new JdkLoggerAdapter()); - } else if ("log4j2".equals(logger)) { - setLoggerAdapter(new Log4j2LoggerAdapter()); - } else { - try { + switch (logger) { + case "slf4j": + setLoggerAdapter(new Slf4jLoggerAdapter()); + break; + case "jcl": + setLoggerAdapter(new JclLoggerAdapter()); + break; + case "log4j": setLoggerAdapter(new Log4jLoggerAdapter()); - } catch (Throwable e1) { - try { - setLoggerAdapter(new Slf4jLoggerAdapter()); - } catch (Throwable e2) { + break; + case "jdk": + setLoggerAdapter(new JdkLoggerAdapter()); + break; + case "log4j2": + setLoggerAdapter(new Log4j2LoggerAdapter()); + break; + default: + List> candidates = Arrays.asList( + Log4jLoggerAdapter.class, + Slf4jLoggerAdapter.class, + Log4j2LoggerAdapter.class, + JclLoggerAdapter.class, + JdkLoggerAdapter.class + ); + for (Class clazz : candidates) { try { - setLoggerAdapter(new Log4j2LoggerAdapter()); - } catch (Throwable e3) { - try { - setLoggerAdapter(new JclLoggerAdapter()); - } catch (Throwable e4) { - setLoggerAdapter(new JdkLoggerAdapter()); - } + setLoggerAdapter(clazz.newInstance()); + } catch (Exception ignored) { } } - } + break; } } @@ -153,4 +159,4 @@ public static File getFile() { return LOGGER_ADAPTER.getFile(); } -} \ No newline at end of file +} From e1a540bdf9dc10d2390e3835064d24b23079d9ff Mon Sep 17 00:00:00 2001 From: kezhenxu94 Date: Wed, 30 Jan 2019 10:36:32 +0800 Subject: [PATCH 2/7] polishing code using map.computeIfAbsent --- .../dubbo/common/logger/LoggerFactory.java | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java index e4f715fd3ee4..a6aea9dd0fb1 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java @@ -20,7 +20,6 @@ import org.apache.dubbo.common.logger.jcl.JclLoggerAdapter; import org.apache.dubbo.common.logger.jdk.JdkLoggerAdapter; import org.apache.dubbo.common.logger.log4j.Log4jLoggerAdapter; -import org.apache.dubbo.common.logger.log4j2.Log4j2Logger; import org.apache.dubbo.common.logger.log4j2.Log4j2LoggerAdapter; import org.apache.dubbo.common.logger.slf4j.Slf4jLoggerAdapter; import org.apache.dubbo.common.logger.support.FailsafeLogger; @@ -37,7 +36,7 @@ */ public class LoggerFactory { - private static final ConcurrentMap LOGGERS = new ConcurrentHashMap(); + private static final ConcurrentMap LOGGERS = new ConcurrentHashMap<>(); private static volatile LoggerAdapter LOGGER_ADAPTER; // search common-used logging frameworks @@ -109,12 +108,7 @@ public static void setLoggerAdapter(LoggerAdapter loggerAdapter) { * @return logger */ public static Logger getLogger(Class key) { - FailsafeLogger logger = LOGGERS.get(key.getName()); - if (logger == null) { - LOGGERS.putIfAbsent(key.getName(), new FailsafeLogger(LOGGER_ADAPTER.getLogger(key))); - logger = LOGGERS.get(key.getName()); - } - return logger; + return LOGGERS.computeIfAbsent(key.getName(), name -> new FailsafeLogger(LOGGER_ADAPTER.getLogger(key))); } /** @@ -124,12 +118,7 @@ public static Logger getLogger(Class key) { * @return logger provider */ public static Logger getLogger(String key) { - FailsafeLogger logger = LOGGERS.get(key); - if (logger == null) { - LOGGERS.putIfAbsent(key, new FailsafeLogger(LOGGER_ADAPTER.getLogger(key))); - logger = LOGGERS.get(key); - } - return logger; + return LOGGERS.computeIfAbsent(key, k -> new FailsafeLogger(LOGGER_ADAPTER.getLogger(k))); } /** From bc652eb9dd453c1360eae6563d63db6f284997d8 Mon Sep 17 00:00:00 2001 From: kezhenxu94 Date: Wed, 30 Jan 2019 12:49:03 +0800 Subject: [PATCH 3/7] fix ci failure --- .../java/org/apache/dubbo/common/logger/LoggerFactory.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java index a6aea9dd0fb1..7c50bdd7971f 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java @@ -41,7 +41,7 @@ public class LoggerFactory { // search common-used logging frameworks static { - String logger = System.getProperty("dubbo.application.logger"); + String logger = System.getProperty("dubbo.application.logger", ""); switch (logger) { case "slf4j": setLoggerAdapter(new Slf4jLoggerAdapter()); @@ -69,7 +69,8 @@ public class LoggerFactory { for (Class clazz : candidates) { try { setLoggerAdapter(clazz.newInstance()); - } catch (Exception ignored) { + break; + } catch (Throwable ignored) { } } break; From 550e8a6de57c217d384c873069400762ade2a986 Mon Sep 17 00:00:00 2001 From: kezhenxu94 Date: Wed, 30 Jan 2019 14:14:16 +0800 Subject: [PATCH 4/7] remove unnecessary break in switch --- .../main/java/org/apache/dubbo/common/logger/LoggerFactory.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java index 7c50bdd7971f..5204d287bbff 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java @@ -73,7 +73,6 @@ public class LoggerFactory { } catch (Throwable ignored) { } } - break; } } From 5b3c821e384979d6cbe94f2613ea5d49cc26c4bc Mon Sep 17 00:00:00 2001 From: kezhenxu94 Date: Wed, 30 Jan 2019 23:34:06 +0800 Subject: [PATCH 5/7] call overloaded method --- .../main/java/org/apache/dubbo/common/logger/LoggerFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java index 5204d287bbff..b1de1795fc39 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java @@ -108,7 +108,7 @@ public static void setLoggerAdapter(LoggerAdapter loggerAdapter) { * @return logger */ public static Logger getLogger(Class key) { - return LOGGERS.computeIfAbsent(key.getName(), name -> new FailsafeLogger(LOGGER_ADAPTER.getLogger(key))); + return getLogger(key.getName()); } /** From 5188b281bf88f6e1d6cb03ca4e60d18d114e657b Mon Sep 17 00:00:00 2001 From: kezhenxu94 Date: Thu, 31 Jan 2019 10:00:27 +0800 Subject: [PATCH 6/7] update as requested --- .../main/java/org/apache/dubbo/common/logger/LoggerFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java index b1de1795fc39..41d1632eacce 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/LoggerFactory.java @@ -108,7 +108,7 @@ public static void setLoggerAdapter(LoggerAdapter loggerAdapter) { * @return logger */ public static Logger getLogger(Class key) { - return getLogger(key.getName()); + return LOGGERS.computeIfAbsent(key.getName(), name -> new FailsafeLogger(LOGGER_ADAPTER.getLogger(name))); } /** From 50ad261ec78744f014b069f294e10942229f282e Mon Sep 17 00:00:00 2001 From: kezhenxu94 Date: Thu, 31 Jan 2019 10:18:14 +0800 Subject: [PATCH 7/7] add unit test --- .../apache/dubbo/common/logger/LoggerFactoryTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/logger/LoggerFactoryTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/logger/LoggerFactoryTest.java index b331f7683c8e..25fce0ebdfdd 100644 --- a/dubbo-common/src/test/java/org/apache/dubbo/common/logger/LoggerFactoryTest.java +++ b/dubbo-common/src/test/java/org/apache/dubbo/common/logger/LoggerFactoryTest.java @@ -58,4 +58,12 @@ public void testGetLogger() { assertThat(logger1, is(logger2)); } -} \ No newline at end of file + + @Test + public void shouldReturnSameLogger() { + Logger logger1 = LoggerFactory.getLogger(this.getClass().getName()); + Logger logger2 = LoggerFactory.getLogger(this.getClass().getName()); + + assertThat(logger1, is(logger2)); + } +}