Skip to content

Commit

Permalink
fix: Explicit provider fails when used with module system
Browse files Browse the repository at this point in the history
this fixes #392

Signed-off-by: Robert Scholte <[email protected]>
  • Loading branch information
rfscholte committed Feb 4, 2024
1 parent 8c4ea8f commit 2c52f5f
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 32 deletions.
58 changes: 38 additions & 20 deletions slf4j-api/src/main/java/org/slf4j/LoggerFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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);
Expand Down
15 changes: 3 additions & 12 deletions slf4j-api/src/test/java/org/slf4j/LoggerFactoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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"));
}

Expand Down

0 comments on commit 2c52f5f

Please sign in to comment.