Polish

qos-ch · Aug 10, 2021 · 258caf4 · 258caf4
1 parent b67dabb
commit 258caf4
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 22 deletions.
diff --git a/slf4j-jdk-platform-logging/src/main/java/org/slf4j/jdk/SLF4JSystemLogger.java b/slf4j-jdk-platform-logging/src/main/java/org/slf4j/jdk/SLF4JSystemLogger.java
@@ -1,14 +1,19 @@
 package org.slf4j.jdk;
 
 import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import java.text.MessageFormat;
 import java.util.ResourceBundle;
 
 import static java.util.Objects.requireNonNull;
 
+/**
+ * Adapts {@link Logger} to {@link System.Logger}.
+ */
 class SLF4JSystemLogger implements System.Logger {
 
+    private static final Logger INTERNAL_LOGGER = LoggerFactory.getLogger(SLF4JSystemLogger.class);
+
     private final Logger logger;
 
     public SLF4JSystemLogger(Logger logger) {
@@ -24,7 +29,8 @@ public String getName() {
     public boolean isLoggable(Level level) {
         switch(level) {
             case ALL:
-                throw new UnsupportedOperationException();
+                // fall-through intended because `ALL` is loggable if the
+                // lowest level is enabled
             case TRACE:
                 return logger.isTraceEnabled();
             case DEBUG:
@@ -36,59 +42,74 @@ public boolean isLoggable(Level level) {
             case ERROR:
                 return logger.isErrorEnabled();
             case OFF:
-                throw new UnsupportedOperationException();
+                // all logging is disabled if the highest level is disabled
+                return !logger.isErrorEnabled();
+            default:
+                INTERNAL_LOGGER.error(
+                        "SLF4J internal error: unknown log level {} passed to `isLoggable` (likely by the JDK).", level);
+                return true;
         }
-        // TODO
-        return true;
     }
 
     @Override
     public void log(Level level, ResourceBundle bundle, String msg, Throwable thrown) {
+        String message = bundle == null ? msg : bundle.getString(msg);
         switch(level) {
             case ALL:
-                throw new UnsupportedOperationException();
+                // fall-through intended because a message is visible on all log levels
+                // if it is logged on the lowest level
             case TRACE:
-                logger.trace(msg, thrown);
+                logger.trace(message, thrown);
                 break;
             case DEBUG:
-                logger.debug(msg, thrown);
+                logger.debug(message, thrown);
                 break;
             case INFO:
-                logger.info(msg, thrown);
+                logger.info(message, thrown);
                 break;
             case WARNING:
-                logger.warn(msg, thrown);
+                logger.warn(message, thrown);
                 break;
             case ERROR:
-                logger.error(msg, thrown);
+                logger.error(message, thrown);
                 break;
             case OFF:
-                throw new UnsupportedOperationException();
+                // don't do anything for a message on level `OFF`
+                break;
+            default:
+                INTERNAL_LOGGER.error(
+                        "SLF4J internal error: unknown log level {} passed to `log` (likely by the JDK).", level);
         }
     }
 
     @Override
     public void log(Level level, ResourceBundle bundle, String format, Object... params) {
+        String message = bundle == null ? format : bundle.getString(format);
         switch(level) {
             case ALL:
-                throw new UnsupportedOperationException();
+                // fall-through intended because a message is visible on all log levels
+                // if it is logged on the lowest level
             case TRACE:
-                logger.trace(MessageFormat.format(format, params));
+                logger.trace(message, params);
                 break;
             case DEBUG:
-                logger.debug(MessageFormat.format(format, params));
+                logger.debug(message, params);
                 break;
             case INFO:
-                logger.info(MessageFormat.format(format, params));
+                logger.info(message, params);
                 break;
             case WARNING:
-                logger.warn(MessageFormat.format(format, params));
+                logger.warn(message, params);
                 break;
             case ERROR:
-                logger.error(MessageFormat.format(format, params));
+                logger.error(message, params);
                 break;
             case OFF:
-                throw new UnsupportedOperationException();
+                // don't do anything for a message on level `OFF`
+                break;
+            default:
+                INTERNAL_LOGGER.error(
+                        "SLF4J internal error: unknown log level {} passed to `log` (likely by the JDK).", level);
         }
     }
 

diff --git a/slf4j-jdk-platform-logging/src/main/java/org/slf4j/jdk/SLF4JSystemLoggerFinder.java b/slf4j-jdk-platform-logging/src/main/java/org/slf4j/jdk/SLF4JSystemLoggerFinder.java
@@ -1,14 +1,33 @@
 package org.slf4j.jdk;
 
-import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/**
+ * Uses SLF4J's {@link LoggerFactory#getLogger(String)} to get a logger
+ * that is adapted for {@link System.Logger}.
+ */
 public class SLF4JSystemLoggerFinder extends System.LoggerFinder {
 
     @Override
     public System.Logger getLogger(String name, Module module) {
-        // TODO do we need to use the `module`?
-        Logger slf4JLogger = LoggerFactory.getLogger(name);
+        // JEP 264[1], which introduced the Platform Logging API,
+        // contains the following note:
+        //
+        //  > An implementation of the LoggerFinder service should make it
+        //  > possible to distinguish system loggers (used by system classes
+        //  > from the Bootstrap Class Loader (BCL)) and application loggers
+        //  > (created by an application for its own usage). This distinction
+        //  > is important for platform security. The creator of a logger can
+        //  > pass the class or module for which the logger is created to the
+        //  > LoggerFinder so that the LoggerFinder can figure out which kind
+        //  > of logger to return.
+        //
+        // If backends support the distinction support this distinction and
+        // once `LoggerFactory`'s API is updated to forward a module, we
+        // should do that here.
+        //
+        // [1] https://openjdk.java.net/jeps/264
+        var slf4JLogger = LoggerFactory.getLogger(name);
         return new SLF4JSystemLogger(slf4JLogger);
     }