fix: Explicit provider fails when used with module system

this fixes #392

Signed-off-by: Robert Scholte <rfscholte@apache.org>

Author: rfscholte

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<SLF4JServiceProvider> findServiceProviders() {
-        List<SLF4JServiceProvider> 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<SLF4JServiceProvider> serviceLoader = getServiceLoader(classLoaderOfLoggerFactory);
 
-         ServiceLoader<SLF4JServiceProvider> serviceLoader = getServiceLoader(classLoaderOfLoggerFactory);
+        Stream<Provider<SLF4JServiceProvider>> stream = serviceLoader.stream();
 
-        Iterator<SLF4JServiceProvider> 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<SLF4JServiceProvider> 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<SLF4JServiceProvider> getServiceLoader(final ClassL
         return serviceLoader;
     }
 
-    private static void safelyInstantiate(List<SLF4JServiceProvider> providerList, Iterator<SLF4JServiceProvider> iterator) {
+    /**
+     * 
+     * @param provider
+     * @return the initiated provider or {@code null} if it fails
+     */
+    private static SLF4JServiceProvider safelyInstantiate(Provider<SLF4JServiceProvider> 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);

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,32 +32,24 @@ 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());
 
 
     }
 
-    @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"));
     }