This is an automated email from the ASF dual-hosted git repository. mattsicker pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 0f463a2f8bc31ec5a0a44b24e9f1f6125b8f964e Author: Matt Sicker <[email protected]> AuthorDate: Tue Mar 29 21:19:21 2022 -0500 Reduce duplicate PluginManager::collectPlugins calls This makes an initial collectPlugins call when each category PluginManager is first created. If a binding for a list of strings named PluginPackages is present, then these packages are scanned for plugins in addition to the usual service loading mechanism. Signed-off-by: Matt Sicker <[email protected]> --- .../log4j/core/config/AbstractConfiguration.java | 13 ++++------- .../log4j/core/config/ConfigurationFactory.java | 4 ++++ .../core/config/DefaultConfigurationFactory.java | 8 ++----- .../logging/log4j/core/impl/DefaultCallback.java | 8 ++----- .../logging/log4j/core/lookup/Interpolator.java | 26 ++++++++++++++++------ .../logging/log4j/core/pattern/PatternParser.java | 12 +++++----- .../flume/appender/FlumePersistentManager.java | 10 ++++----- .../log4j/plugins/convert/TypeConverter.java | 5 +++++ .../logging/log4j/plugins/di/DefaultInjector.java | 10 ++++++--- .../org/apache/logging/log4j/plugins/di/Keys.java | 4 ++++ 10 files changed, 58 insertions(+), 42 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java index 266e7af..849b27b 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java @@ -167,7 +167,6 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement tempLookup = interpolatorFactory.newInterpolator(new PropertiesLookup(properties)); runtimeStrSubstitutor = new RuntimeStrSubstitutor(tempLookup); configurationStrSubstitutor = new ConfigurationStrSubstitutor(runtimeStrSubstitutor); - pluginManager = injector.getInstance(Core.PLUGIN_MANAGER_KEY); configurationScheduler = injector.getInstance(ConfigurationScheduler.class); watchManager = injector.getInstance(WatchManager.class); setState(State.INITIALIZING); @@ -241,9 +240,9 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement runtimeStrSubstitutor.setConfiguration(this); configurationStrSubstitutor.setConfiguration(this); initializeScriptManager(); - pluginManager.collectPlugins(pluginPackages); + injector.registerBindingIfAbsent(Keys.PLUGIN_PACKAGES_KEY, this::getPluginPackages); + pluginManager = injector.getInstance(Core.PLUGIN_MANAGER_KEY); final PluginManager levelPlugins = injector.getInstance(new @Named(Level.CATEGORY) Key<>() {}); - levelPlugins.collectPlugins(pluginPackages); final Map<String, PluginType<?>> plugins = levelPlugins.getPlugins(); if (plugins != null) { for (final PluginType<?> type : plugins.values()) { @@ -304,12 +303,8 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement if (configSource.getLastModified() > 0) { final Source cfgSource = new Source(configSource); final Key<WatcherFactory> key = Key.forClass(WatcherFactory.class); - injector.registerBindingIfAbsent(key, new LazyValue<>(() -> { - final PluginManager pluginManager = injector.getInstance(Watcher.PLUGIN_MANAGER_KEY); - pluginManager.collectPlugins(pluginPackages); - final Map<String, PluginType<?>> watcherPlugins = pluginManager.getPlugins(); - return new WatcherFactory(watcherPlugins); - })); + injector.registerBindingIfAbsent(key, LazyValue.from(() -> + new WatcherFactory(injector.getInstance(Watcher.PLUGIN_MANAGER_KEY).getPlugins()))); final Watcher watcher = injector.getInstance(key) .newWatcher(cfgSource, this, reconfigurable, listeners, configSource.getLastModified()); if (watcher != null) { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java index fc204fa..fac845f 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java @@ -26,8 +26,10 @@ import org.apache.logging.log4j.core.util.AuthorizationProvider; import org.apache.logging.log4j.core.util.BasicAuthorizationProvider; import org.apache.logging.log4j.core.util.FileUtils; import org.apache.logging.log4j.plugins.Inject; +import org.apache.logging.log4j.plugins.Named; import org.apache.logging.log4j.plugins.di.Injector; import org.apache.logging.log4j.plugins.di.Key; +import org.apache.logging.log4j.plugins.util.PluginManager; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.LoaderUtil; import org.apache.logging.log4j.util.PropertiesUtil; @@ -90,6 +92,8 @@ public abstract class ConfigurationFactory extends ConfigurationBuilderFactory { public static final Key<ConfigurationFactory> KEY = new Key<>() {}; + public static final Key<PluginManager> PLUGIN_MANAGER_KEY = new @Named(CATEGORY) Key<>() {}; + /** * Allows subclasses access to the status logger without creating another instance. */ diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/DefaultConfigurationFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/DefaultConfigurationFactory.java index 4735085..0ec7df5 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/DefaultConfigurationFactory.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/DefaultConfigurationFactory.java @@ -23,10 +23,7 @@ import org.apache.logging.log4j.core.config.composite.CompositeConfiguration; import org.apache.logging.log4j.core.util.Loader; import org.apache.logging.log4j.core.util.NetUtils; import org.apache.logging.log4j.plugins.Inject; -import org.apache.logging.log4j.plugins.Named; import org.apache.logging.log4j.plugins.di.Injector; -import org.apache.logging.log4j.plugins.di.Key; -import org.apache.logging.log4j.plugins.util.PluginManager; import org.apache.logging.log4j.util.LazyValue; import org.apache.logging.log4j.util.LoaderUtil; import org.apache.logging.log4j.util.PropertiesUtil; @@ -314,9 +311,8 @@ public class DefaultConfigurationFactory extends ConfigurationFactory { }) .stream(); - final PluginManager manager = injector.getInstance(new @Named(CATEGORY) Key<>() {}); - manager.collectPlugins(); - final Stream<? extends ConfigurationFactory> pluginConfigurationFactoryStream = manager.getPlugins() + final Stream<? extends ConfigurationFactory> pluginConfigurationFactoryStream = injector.getInstance(PLUGIN_MANAGER_KEY) + .getPlugins() .values() .stream() .flatMap(type -> { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultCallback.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultCallback.java index 590222f..aa83f15 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultCallback.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultCallback.java @@ -46,7 +46,6 @@ import org.apache.logging.log4j.plugins.PluginException; import org.apache.logging.log4j.plugins.di.Injector; import org.apache.logging.log4j.plugins.di.InjectorCallback; import org.apache.logging.log4j.plugins.di.Key; -import org.apache.logging.log4j.plugins.util.PluginManager; import org.apache.logging.log4j.spi.CopyOnWrite; import org.apache.logging.log4j.spi.DefaultThreadContextMap; import org.apache.logging.log4j.spi.ReadOnlyThreadContextMap; @@ -126,11 +125,8 @@ public class DefaultCallback implements InjectorCallback { () -> Constants.ENABLE_THREADLOCALS ? ReusableLogEventFactory.class : DefaultLogEventFactory.class)) .registerBindingIfAbsent(Key.forClass(InterpolatorFactory.class), - () -> defaultLookup -> { - final PluginManager pluginManager = injector.getInstance(StrLookup.PLUGIN_MANAGER_KEY); - pluginManager.collectPlugins(); - return new Interpolator(defaultLookup, pluginManager.getPlugins(), injector::getInstance); - }) + () -> defaultLookup -> new Interpolator(defaultLookup, + injector.getInstance(StrLookup.PLUGIN_MANAGER_KEY).getPlugins(), injector::getInstance)) .registerBindingIfAbsent(Key.forClass(StrSubstitutor.class), () -> new StrSubstitutor(injector.getInstance(InterpolatorFactory.class).newInterpolator(null))) .registerBindingIfAbsent(ConfigurationFactory.KEY, injector.getFactory(DefaultConfigurationFactory.class)) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/Interpolator.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/Interpolator.java index e238826..387b0e0 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/Interpolator.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/Interpolator.java @@ -19,11 +19,12 @@ package org.apache.logging.log4j.core.lookup; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.config.ConfigurationAware; +import org.apache.logging.log4j.plugins.di.DI; +import org.apache.logging.log4j.plugins.di.Injector; +import org.apache.logging.log4j.plugins.di.Keys; import org.apache.logging.log4j.plugins.util.PluginType; -import org.apache.logging.log4j.plugins.util.PluginUtil; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.LazyValue; -import org.apache.logging.log4j.util.ReflectionUtil; import java.util.List; import java.util.Locale; @@ -55,7 +56,7 @@ public class Interpolator extends AbstractConfigurationAwareLookup { private static final Logger LOGGER = StatusLogger.getLogger(); - private final Map<String, Supplier<StrLookup>> strLookups = new ConcurrentHashMap<>(); + private final Map<String, Supplier<? extends StrLookup>> strLookups = new ConcurrentHashMap<>(); private final StrLookup defaultLookup; @@ -71,7 +72,19 @@ public class Interpolator extends AbstractConfigurationAwareLookup { * @since 2.1 */ public Interpolator(final StrLookup defaultLookup, final List<String> pluginPackages) { - this(defaultLookup, PluginUtil.collectPluginsByCategoryAndPackage(CATEGORY, pluginPackages), ReflectionUtil::instantiate); + this.defaultLookup = defaultLookup == null ? new PropertiesLookup(Map.of()) : defaultLookup; + final Injector injector = DI.createInjector(); + injector.registerBinding(Keys.PLUGIN_PACKAGES_KEY, () -> pluginPackages); + injector.getInstance(PLUGIN_MANAGER_KEY) + .getPlugins() + .forEach((key, value) -> { + try { + strLookups.put(key.toLowerCase(Locale.ROOT), + injector.getFactory(value.getPluginClass().asSubclass(StrLookup.class))); + } catch (final Throwable t) { + handleError(key, t); + } + }); } public Interpolator( @@ -81,8 +94,7 @@ public class Interpolator extends AbstractConfigurationAwareLookup { strLookupPlugins.forEach((key, value) -> { try { final Class<? extends StrLookup> strLookupClass = value.getPluginClass().asSubclass(StrLookup.class); - final Supplier<StrLookup> strLookupSupplier = LazyValue.from(() -> pluginLoader.apply(strLookupClass)); - strLookups.put(key.toLowerCase(Locale.ROOT), strLookupSupplier); + strLookups.put(key.toLowerCase(Locale.ROOT), LazyValue.from(() -> pluginLoader.apply(strLookupClass))); } catch (final Throwable t) { handleError(key, t); } @@ -167,7 +179,7 @@ public class Interpolator extends AbstractConfigurationAwareLookup { if (prefixPos >= 0) { final String prefix = var.substring(0, prefixPos).toLowerCase(Locale.US); final String name = var.substring(prefixPos + 1); - final Supplier<StrLookup> lookupSupplier = strLookups.get(prefix); + final Supplier<? extends StrLookup> lookupSupplier = strLookups.get(prefix); String value = null; if (lookupSupplier != null) { final StrLookup lookup = lookupSupplier.get(); diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java index 4244841..03627b6 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java @@ -20,10 +20,10 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.time.SystemNanoClock; import org.apache.logging.log4j.plugins.Named; +import org.apache.logging.log4j.plugins.di.DI; import org.apache.logging.log4j.plugins.di.Key; import org.apache.logging.log4j.plugins.util.PluginManager; import org.apache.logging.log4j.plugins.util.PluginType; -import org.apache.logging.log4j.plugins.util.PluginUtil; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.Strings; @@ -88,6 +88,8 @@ public final class PatternParser { private static final int DECIMAL = 10; + private static final Key<PluginManager> PLUGIN_MANAGER_KEY = Key.forClass(PluginManager.class).withQualifierType(Named.class); + private final Configuration config; private final Map<String, Class<? extends PatternConverter>> converterRules; @@ -132,13 +134,11 @@ public final class PatternParser { final Class<?> filterClass) { this.config = config; final Map<String, PluginType<?>> plugins; + final Key<PluginManager> pluginManagerKey = PLUGIN_MANAGER_KEY.withName(converterKey); if (config == null) { - plugins = PluginUtil.collectPluginsByCategory(converterKey); + plugins = DI.createInjector().getInstance(pluginManagerKey).getPlugins(); } else { - final PluginManager manager = config.getComponent( - Key.forClass(PluginManager.class).withName(converterKey).withQualifierType(Named.class)); - manager.collectPlugins(config.getPluginPackages()); - plugins = manager.getPlugins(); + plugins = config.getComponent(pluginManagerKey).getPlugins(); } final Map<String, Class<? extends PatternConverter>> converters = new LinkedHashMap<>(); diff --git a/log4j-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumePersistentManager.java b/log4j-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumePersistentManager.java index b802f0c..8ddf166 100644 --- a/log4j-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumePersistentManager.java +++ b/log4j-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumePersistentManager.java @@ -39,7 +39,6 @@ import org.apache.logging.log4j.core.util.Log4jThread; import org.apache.logging.log4j.core.util.Log4jThreadFactory; import org.apache.logging.log4j.core.util.SecretKeyProvider; import org.apache.logging.log4j.plugins.di.Injector; -import org.apache.logging.log4j.plugins.util.PluginManager; import org.apache.logging.log4j.plugins.util.PluginType; import org.apache.logging.log4j.util.Strings; @@ -433,9 +432,9 @@ public class FlumePersistentManager extends FlumeAvroManager { } } if (key != null) { - final PluginManager pluginManager = data.injector.getInstance(SecretKeyProvider.PLUGIN_MANAGER_KEY); - pluginManager.collectPlugins(); - final Map<String, PluginType<?>> plugins = pluginManager.getPlugins(); + final Injector injector = data.injector; + final Map<String, PluginType<?>> plugins = + injector.getInstance(SecretKeyProvider.PLUGIN_MANAGER_KEY).getPlugins(); if (plugins != null) { boolean found = false; for (final Map.Entry<String, PluginType<?>> entry : plugins.entrySet()) { @@ -443,7 +442,8 @@ public class FlumePersistentManager extends FlumeAvroManager { found = true; final Class<?> cl = entry.getValue().getPluginClass(); try { - final SecretKeyProvider provider = data.injector.getInstance(cl.asSubclass(SecretKeyProvider.class)); + final SecretKeyProvider provider = + injector.getInstance(cl.asSubclass(SecretKeyProvider.class)); secretKey = provider.getSecretKey(); LOGGER.debug("Persisting events using SecretKeyProvider {}", cl.getName()); } catch (final Exception ex) { diff --git a/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/convert/TypeConverter.java b/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/convert/TypeConverter.java index 403ab57..781f321 100644 --- a/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/convert/TypeConverter.java +++ b/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/convert/TypeConverter.java @@ -17,7 +17,10 @@ package org.apache.logging.log4j.plugins.convert; +import org.apache.logging.log4j.plugins.Named; import org.apache.logging.log4j.plugins.Plugin; +import org.apache.logging.log4j.plugins.di.Key; +import org.apache.logging.log4j.plugins.util.PluginManager; import org.apache.logging.log4j.plugins.util.TypeUtil; import org.apache.logging.log4j.status.StatusLogger; @@ -35,6 +38,8 @@ public interface TypeConverter<T> { */ String CATEGORY = "TypeConverter"; + Key<PluginManager> PLUGIN_MANAGER_KEY = new @Named(CATEGORY) Key<>() {}; + /** * Converts a String to a given type. * diff --git a/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/DefaultInjector.java b/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/DefaultInjector.java index 6d62cce..4ed098f 100644 --- a/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/DefaultInjector.java +++ b/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/DefaultInjector.java @@ -250,7 +250,12 @@ class DefaultInjector implements Injector { final Class<T> rawType = key.getRawType(); final Scope scope = getScopeForType(rawType); if (rawType == PluginManager.class && key.getQualifierType() == Named.class) { - final Supplier<T> factory = () -> TypeUtil.cast(new PluginManager(key.getName())); + final Supplier<T> factory = () -> { + final var manager = new PluginManager(key.getName()); + final Binding<List<String>> pluginPackagesBinding = bindingMap.get(Keys.PLUGIN_PACKAGES_KEY, List.of()); + manager.collectPlugins(pluginPackagesBinding != null ? pluginPackagesBinding.getSupplier().get() : List.of()); + return TypeUtil.cast(manager); + }; bindingMap.put(key, scope.get(key, factory)); return bindingMap.get(key, aliases).getSupplier(); } @@ -293,8 +298,7 @@ class DefaultInjector implements Injector { } private void initializeTypeConverters() { - final PluginManager manager = getInstance(new @Named(TypeConverter.CATEGORY) Key<>() {}); - manager.collectPlugins(); + final PluginManager manager = getInstance(TypeConverter.PLUGIN_MANAGER_KEY); for (final PluginType<?> knownType : manager.getPlugins().values()) { final Class<?> pluginClass = knownType.getPluginClass(); final Type type = getTypeConverterSupportedType(pluginClass); diff --git a/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/Keys.java b/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/Keys.java index 1b56046..3b06c63 100644 --- a/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/Keys.java +++ b/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/Keys.java @@ -19,6 +19,7 @@ package org.apache.logging.log4j.plugins.di; import org.apache.logging.log4j.plugins.Named; +import java.util.List; import java.util.function.Function; public final class Keys { @@ -28,4 +29,7 @@ public final class Keys { public static final String SUBSTITUTOR_NAME = "StringSubstitutor"; public static final Key<Function<String, String>> SUBSTITUTOR_KEY = new @Named(SUBSTITUTOR_NAME) Key<>() {}; + + public static final String PLUGIN_PACKAGES_NAME = "PluginPackages"; + public static final Key<List<String>> PLUGIN_PACKAGES_KEY = new @Named(PLUGIN_PACKAGES_NAME) Key<>() {}; }
