Enhancement/logger factory (#3389)

* polishing LoggerFactory * polishing code using map.computeIfAbsent * fix ci failure * remove unnecessary break in switch * call overloaded method * update as requested * add unit test
apache · Jan 31, 2019 · 342f37a · 342f37a
1 parent 35f1914
commit 342f37a
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 40 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
@@ -25,6 +25,8 @@
 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;
@@ -34,40 +36,43 @@
  */
 public class LoggerFactory {
 
-    private static final ConcurrentMap<String, FailsafeLogger> LOGGERS = new ConcurrentHashMap<String, FailsafeLogger>();
+    private static final ConcurrentMap<String, FailsafeLogger> LOGGERS = new ConcurrentHashMap<>();
     private static volatile LoggerAdapter LOGGER_ADAPTER;
 
     // 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 {
+        String logger = System.getProperty("dubbo.application.logger", "");
+        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<Class<? extends LoggerAdapter>> candidates = Arrays.asList(
+                        Log4jLoggerAdapter.class,
+                        Slf4jLoggerAdapter.class,
+                        Log4j2LoggerAdapter.class,
+                        JclLoggerAdapter.class,
+                        JdkLoggerAdapter.class
+                );
+                for (Class<? extends LoggerAdapter> clazz : candidates) {
                     try {
-                        setLoggerAdapter(new Log4j2LoggerAdapter());
-                    } catch (Throwable e3) {
-                        try {
-                            setLoggerAdapter(new JclLoggerAdapter());
-                        } catch (Throwable e4) {
-                            setLoggerAdapter(new JdkLoggerAdapter());
-                        }
+                        setLoggerAdapter(clazz.newInstance());
+                        break;
+                    } catch (Throwable ignored) {
                     }
                 }
-            }
         }
     }
 
@@ -103,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(name)));
     }
 
     /**
@@ -118,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)));
     }
 
     /**
@@ -153,4 +148,4 @@ public static File getFile() {
         return LOGGER_ADAPTER.getFile();
     }
 
-}
+}
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
@@ -58,4 +58,12 @@ public void testGetLogger() {
 
         assertThat(logger1, is(logger2));
     }
-}
+
+    @Test
+    public void shouldReturnSameLogger() {
+        Logger logger1 = LoggerFactory.getLogger(this.getClass().getName());
+        Logger logger2 = LoggerFactory.getLogger(this.getClass().getName());
+
+        assertThat(logger1, is(logger2));
+    }
+}