Remove instance of checks for async components

ppkarwasz · ppkarwasz · commit 4e732f1097b8 · 2024-01-29T20:01:54.000+01:00
This PR removes all occurences of `instanceof Async*` that might prevent
the separation of the async support.

The removal requires a couple of new API methods and binary incompatible
changes.
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
@@ -939,4 +939,11 @@ public String toString() {
     protected Logger newInstance(final LoggerContext ctx, final String name, final MessageFactory messageFactory) {
         return new Logger(ctx, name, messageFactory);
     }
+
+    /**
+     * If {@code true} loggers will include location by default.
+     */
+    public boolean includeLocation() {
+        return true;
+    }
 }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerConfig.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerConfig.java
@@ -18,6 +18,7 @@
 
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.core.Filter;
@@ -79,6 +80,7 @@ public LoggerConfig build() {
             final String name = getLoggerName().equals(ROOT) ? Strings.EMPTY : getLoggerName();
             final LevelAndRefs container =
                     LoggerConfig.getLevelAndRefs(getLevel(), getRefs(), getLevelAndRefs(), getConfig());
+            final String includeLocationConfigValue = getIncludeLocation();
             return new AsyncLoggerConfig(
                     name,
                     container.refs,
@@ -87,7 +89,7 @@ public LoggerConfig build() {
                     isAdditivity(),
                     getProperties(),
                     getConfig(),
-                    includeLocation(getIncludeLocation()),
+                    Boolean.parseBoolean(includeLocationConfigValue),
                     getLogEventFactory());
         }
     }
@@ -116,9 +118,9 @@ public void initialize() {
         super.initialize();
     }
 
-    protected void log(final LogEvent event, final LoggerConfigPredicate predicate) {
+    protected void log(final LogEvent event, final Predicate<LoggerConfig> predicate) {
         // See LOG4J2-2301
-        if (predicate == LoggerConfigPredicate.ALL
+        if (predicate == null
                 && ASYNC_LOGGER_ENTERED.get() == Boolean.FALSE
                 &&
                 // Optimization: AsyncLoggerConfig is identical to LoggerConfig
@@ -132,7 +134,7 @@ protected void log(final LogEvent event, final LoggerConfigPredicate predicate)
                 if (!isFiltered(event)) {
                     // Detect the first time we encounter an AsyncLoggerConfig. We must log
                     // to all non-async loggers first.
-                    processLogEvent(event, LoggerConfigPredicate.SYNCHRONOUS_ONLY);
+                    processLogEvent(event, lc -> !(lc instanceof AsyncLoggerConfig));
                     // Then pass the event to the background thread where
                     // all async logging is executed. It is important this
                     // happens at most once and after all synchronous loggers
@@ -206,7 +208,7 @@ void logInBackgroundThread(final LogEvent event) {
      */
     void logToAsyncLoggerConfigsOnCurrentThread(final LogEvent event) {
         // skip the filter, which was already called on the logging thread
-        processLogEvent(event, LoggerConfigPredicate.ASYNCHRONOUS_ONLY);
+        processLogEvent(event, lc -> lc instanceof AsyncLoggerConfig);
     }
 
     private String displayName() {
@@ -228,11 +230,6 @@ public boolean stop(final long timeout, final TimeUnit timeUnit) {
         return true;
     }
 
-    // Note: for asynchronous loggers, includeLocation default is FALSE
-    protected static boolean includeLocation(final String includeLocationConfigValue) {
-        return Boolean.parseBoolean(includeLocationConfigValue);
-    }
-
     /**
      * An asynchronous root Logger.
      */
@@ -251,6 +248,7 @@ public static class Builder<B extends Builder<B>> extends RootLogger.Builder<B>
             public LoggerConfig build() {
                 final LevelAndRefs container =
                         LoggerConfig.getLevelAndRefs(getLevel(), getRefs(), getLevelAndRefs(), getConfig());
+                final String includeLocationConfigValue = getIncludeLocation();
                 return new AsyncLoggerConfig(
                         LogManager.ROOT_LOGGER_NAME,
                         container.refs,
@@ -259,7 +257,7 @@ public LoggerConfig build() {
                         isAdditivity(),
                         getProperties(),
                         getConfig(),
-                        AsyncLoggerConfig.includeLocation(getIncludeLocation()),
+                        Boolean.parseBoolean(includeLocationConfigValue),
                         getLogEventFactory());
             }
         }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerContext.java
@@ -130,6 +130,11 @@ public boolean stop(final long timeout, final TimeUnit timeUnit) {
         return true;
     }
 
+    @Override
+    public boolean includeLocation() {
+        return false;
+    }
+
     // package-protected for tests
     AsyncLoggerDisruptor getAsyncLoggerDisruptor() {
         return loggerDisruptor;
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerContextSelector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerContextSelector.java
@@ -18,12 +18,10 @@
 
 import java.net.URI;
 import org.apache.logging.log4j.core.LoggerContext;
-import org.apache.logging.log4j.core.impl.Log4jPropertyKey;
 import org.apache.logging.log4j.core.selector.ClassLoaderContextSelector;
 import org.apache.logging.log4j.plugins.Inject;
 import org.apache.logging.log4j.plugins.Singleton;
 import org.apache.logging.log4j.plugins.di.ConfigurableInstanceFactory;
-import org.apache.logging.log4j.util.PropertiesUtil;
 
 /**
  * {@code ContextSelector} that manages {@code AsyncLoggerContext} instances.
@@ -33,19 +31,6 @@
 @Singleton
 public class AsyncLoggerContextSelector extends ClassLoaderContextSelector {
 
-    /**
-     * Returns {@code true} if the user specified this selector as the Log4jContextSelector, to make all loggers
-     * asynchronous.
-     *
-     * @return {@code true} if all loggers are asynchronous, {@code false} otherwise.
-     */
-    public static boolean isSelected() {
-        // FIXME(ms): this should check Injector bindings
-        return AsyncLoggerContextSelector.class
-                .getName()
-                .equals(PropertiesUtil.getProperties().getStringProperty(Log4jPropertyKey.CONTEXT_SELECTOR_CLASS_NAME));
-    }
-
     @Inject
     public AsyncLoggerContextSelector(final ConfigurableInstanceFactory instanceFactory) {
         super(instanceFactory);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java
@@ -20,16 +20,14 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Predicate;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Marker;
 import org.apache.logging.log4j.core.Appender;
 import org.apache.logging.log4j.core.Filter;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.LoggerContext;
-import org.apache.logging.log4j.core.async.AsyncLoggerConfig;
-import org.apache.logging.log4j.core.async.AsyncLoggerContext;
-import org.apache.logging.log4j.core.async.AsyncLoggerContextSelector;
 import org.apache.logging.log4j.core.config.plugins.PluginConfiguration;
 import org.apache.logging.log4j.core.filter.AbstractFilterable;
 import org.apache.logging.log4j.core.impl.Log4jLogEvent;
@@ -495,7 +493,7 @@ public void log(
         final LogEvent logEvent =
                 logEventFactory.createEvent(loggerName, marker, fqcn, location(fqcn), level, data, props, t);
         try {
-            log(logEvent, LoggerConfigPredicate.ALL);
+            log(logEvent, null);
         } finally {
             // LOG4J2-1583 prevent scrambled logs when logging calls are nested (logging in toString())
             logEventFactory.recycle(logEvent);
@@ -530,7 +528,7 @@ public void log(
         final LogEvent logEvent =
                 logEventFactory.createEvent(loggerName, marker, fqcn, location, level, data, props, t);
         try {
-            log(logEvent, LoggerConfigPredicate.ALL);
+            log(logEvent, null);
         } finally {
             // LOG4J2-1583 prevent scrambled logs when logging calls are nested (logging in toString())
             logEventFactory.recycle(logEvent);
@@ -584,17 +582,17 @@ private List<Property> getPropertiesWithLookups(
      * @param event The log event.
      */
     public void log(final LogEvent event) {
-        log(event, LoggerConfigPredicate.ALL);
+        log(event, null);
     }
 
     /**
      * Logs an event.
      *
      * @param event     The log event.
-     * @param predicate predicate for which LoggerConfig instances to append to. A
-     *                  {@literal null} value is equivalent to a true predicate.
+     * @param predicate predicate for which LoggerConfig instances to append to.
+     *                  Use a {@literal null} value instead of a true predicate.
      */
-    protected void log(final LogEvent event, final LoggerConfigPredicate predicate) {
+    protected void log(final LogEvent event, final Predicate<LoggerConfig> predicate) {
         if (!isFiltered(event)) {
             processLogEvent(event, predicate);
         }
@@ -614,12 +612,12 @@ public ReliabilityStrategy getReliabilityStrategy() {
      * Logs an event, bypassing filters.
      *
      * @param event     The log event.
-     * @param predicate predicate for which LoggerConfig instances to append to. A
-     *                  {@literal null} value is equivalent to a true predicate.
+     * @param predicate predicate for which LoggerConfig instances to append to.
+     *                  Use a {@literal null} value instead of a true predicate.
      */
-    protected void processLogEvent(final LogEvent event, final LoggerConfigPredicate predicate) {
+    protected void processLogEvent(final LogEvent event, final Predicate<LoggerConfig> predicate) {
         event.setIncludeLocation(isIncludeLocation());
-        if (predicate == null || predicate.allow(this)) {
+        if (predicate == null || predicate.test(this)) {
             callAppenders(event);
         }
         logParent(event, predicate);
@@ -649,7 +647,7 @@ public boolean requiresLocation() {
         return false;
     }
 
-    private void logParent(final LogEvent event, final LoggerConfigPredicate predicate) {
+    private void logParent(final LogEvent event, final Predicate<LoggerConfig> predicate) {
         if (additive && parent != null) {
             parent.log(event, predicate);
         }
@@ -674,14 +672,11 @@ public String toString() {
     protected static boolean includeLocation(
             final String includeLocationConfigValue, final Configuration configuration) {
         if (includeLocationConfigValue == null) {
-            LoggerContext context = null;
             if (configuration != null) {
-                context = configuration.getLoggerContext();
-            }
-            if (context != null) {
-                return !(context instanceof AsyncLoggerContext);
+                final LoggerContext context = configuration.getLoggerContext();
+                return context != null ? context.includeLocation() : false;
             } else {
-                return !AsyncLoggerContextSelector.isSelected();
+                return false;
             }
         }
         return Boolean.parseBoolean(includeLocationConfigValue);
@@ -860,27 +855,4 @@ protected static class LevelAndRefs {
         public Level level;
         public List<AppenderRef> refs;
     }
-
-    protected enum LoggerConfigPredicate {
-        ALL() {
-            @Override
-            boolean allow(final LoggerConfig config) {
-                return true;
-            }
-        },
-        ASYNCHRONOUS_ONLY() {
-            @Override
-            boolean allow(final LoggerConfig config) {
-                return config instanceof AsyncLoggerConfig;
-            }
-        },
-        SYNCHRONOUS_ONLY() {
-            @Override
-            boolean allow(final LoggerConfig config) {
-                return !ASYNCHRONOUS_ONLY.allow(config);
-            }
-        };
-
-        abstract boolean allow(LoggerConfig config);
-    }
 }

Original file line number	Diff line number	Diff line change
`@@ -939,4 +939,11 @@ public String toString() {`
`939`	`939`	`protected Logger newInstance(final LoggerContext ctx, final String name, final MessageFactory messageFactory) {`
`940`	`940`	`return new Logger(ctx, name, messageFactory);`
`941`	`941`	`}`
	`942`	`+`
	`943`	`+ /**`
	`944`	`+ * If {@code true} loggers will include location by default.`
	`945`	`+ */`
	`946`	`+ public boolean includeLocation() {`
	`947`	`+ return true;`
	`948`	`+ }`
`942`	`949`	`}`