From 5026eb4673e73cbb16f2984bd0db758f4a42993b Mon Sep 17 00:00:00 2001 From: Robert Scholte Date: Sun, 4 Feb 2024 15:28:15 +0100 Subject: [PATCH] fix: Explicit provider fails when used with module system this fixes #392 --- .../main/java/org/slf4j/LoggerFactory.java | 58 ++++++++++++------- .../java/org/slf4j/LoggerFactoryTest.java | 15 +---- 2 files changed, 41 insertions(+), 32 deletions(-) diff --git a/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java b/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java index 4e1f0b0d6..d74382537 100755 --- a/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java +++ b/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java @@ -33,13 +33,16 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Enumeration; -import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; +import java.util.Objects; import java.util.ServiceConfigurationError; import java.util.ServiceLoader; +import java.util.ServiceLoader.Provider; import java.util.Set; import java.util.concurrent.LinkedBlockingQueue; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.slf4j.event.SubstituteLoggingEvent; import org.slf4j.helpers.NOP_FallbackServiceProvider; @@ -110,24 +113,32 @@ public final class LoggerFactory { // Package access for tests static List findServiceProviders() { - List providerList = new ArrayList<>(); // retain behaviour similar to that of 1.7 series and earlier. More specifically, use the class loader that // loaded the present class to search for services final ClassLoader classLoaderOfLoggerFactory = LoggerFactory.class.getClassLoader(); - SLF4JServiceProvider explicitProvider = loadExplicitlySpecified(classLoaderOfLoggerFactory); - if(explicitProvider != null) { - providerList.add(explicitProvider); - return providerList; - } - + + ServiceLoader serviceLoader = getServiceLoader(classLoaderOfLoggerFactory); - ServiceLoader serviceLoader = getServiceLoader(classLoaderOfLoggerFactory); + Stream> stream = serviceLoader.stream(); - Iterator iterator = serviceLoader.iterator(); - while (iterator.hasNext()) { - safelyInstantiate(providerList, iterator); + String explicitlySpecified = System.getProperty(PROVIDER_PROPERTY_KEY, ""); + + if (!explicitlySpecified.isEmpty()) { + stream = stream.filter(s -> s.type().getName().equals(explicitlySpecified)); + } + + List providerList = stream.map(LoggerFactory::safelyInstantiate) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + + if(providerList.isEmpty() && !explicitlySpecified.isEmpty()) + { + SLF4JServiceProvider explicitProvider = loadExplicitlySpecified(explicitlySpecified, classLoaderOfLoggerFactory); + if(explicitProvider != null) { + return Arrays.asList(explicitProvider); + } } return providerList; } @@ -144,13 +155,18 @@ private static ServiceLoader getServiceLoader(final ClassL return serviceLoader; } - private static void safelyInstantiate(List providerList, Iterator iterator) { + /** + * + * @param provider + * @return the initiated provider or {@code null} if it fails + */ + private static SLF4JServiceProvider safelyInstantiate(Provider provider) { try { - SLF4JServiceProvider provider = iterator.next(); - providerList.add(provider); + return provider.get(); } catch (ServiceConfigurationError e) { Reporter.error("A service provider failed to instantiate:\n" + e.getMessage()); } + return null; } /** @@ -212,11 +228,13 @@ private final static void bind() { } } - static SLF4JServiceProvider loadExplicitlySpecified(ClassLoader classLoader) { - String explicitlySpecified = System.getProperty(PROVIDER_PROPERTY_KEY); - if (null == explicitlySpecified || explicitlySpecified.isEmpty()) { - return null; - } + /** + * + * @param explicitlySpecified the classname of the provider, never {@code null} + * @param classLoader the classloader, never {@code null} + * @return + */ + static SLF4JServiceProvider loadExplicitlySpecified(String explicitlySpecified, ClassLoader classLoader) { try { String message = String.format("Attempting to load provider \"%s\" specified via \"%s\" system property", explicitlySpecified, PROVIDER_PROPERTY_KEY); Reporter.info(message); diff --git a/slf4j-api/src/test/java/org/slf4j/LoggerFactoryTest.java b/slf4j-api/src/test/java/org/slf4j/LoggerFactoryTest.java index ed618171c..68410b47b 100644 --- a/slf4j-api/src/test/java/org/slf4j/LoggerFactoryTest.java +++ b/slf4j-api/src/test/java/org/slf4j/LoggerFactoryTest.java @@ -9,7 +9,6 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; -import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.*; public class LoggerFactoryTest { @@ -33,8 +32,7 @@ public void cleanUp() { @Test public void testExplicitlySpecified() { - System.setProperty(LoggerFactory.PROVIDER_PROPERTY_KEY, "org.slf4j.LoggerFactoryTest$TestingProvider"); - SLF4JServiceProvider provider = LoggerFactory.loadExplicitlySpecified(classLoaderOfLoggerFactory); + SLF4JServiceProvider provider = LoggerFactory.loadExplicitlySpecified("org.slf4j.LoggerFactoryTest$TestingProvider", classLoaderOfLoggerFactory); assertTrue("provider should be instance of TestingProvider class", provider instanceof TestingProvider); assertTrue(mockedSyserr.toString().contains(" Attempting to load provider \"org.slf4j.LoggerFactoryTest$TestingProvider\" specified via \"slf4j.provider\" system property")); System.out.println(mockedSyserr.toString()); @@ -42,23 +40,16 @@ public void testExplicitlySpecified() { } - @Test - public void testExplicitlySpecifiedNull() { - assertNull(LoggerFactory.loadExplicitlySpecified(classLoaderOfLoggerFactory)); - } - @Test public void testExplicitlySpecifyMissingServiceProvider() { - System.setProperty(LoggerFactory.PROVIDER_PROPERTY_KEY, "com.example.ServiceProvider"); - SLF4JServiceProvider provider = LoggerFactory.loadExplicitlySpecified(classLoaderOfLoggerFactory); + SLF4JServiceProvider provider = LoggerFactory.loadExplicitlySpecified("com.example.ServiceProvider", classLoaderOfLoggerFactory); assertNull(provider); assertTrue(mockedSyserr.toString().contains("Failed to instantiate the specified SLF4JServiceProvider (com.example.ServiceProvider)")); } @Test public void testExplicitlySpecifyNonServiceProvider() { - System.setProperty(LoggerFactory.PROVIDER_PROPERTY_KEY, "java.lang.String"); - assertNull(LoggerFactory.loadExplicitlySpecified(classLoaderOfLoggerFactory)); + assertNull(LoggerFactory.loadExplicitlySpecified("java.lang.String", classLoaderOfLoggerFactory)); assertTrue(mockedSyserr.toString().contains("Specified SLF4JServiceProvider (java.lang.String) does not implement SLF4JServiceProvider interface")); }