Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Explicit provider fails when used with module system #393

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: Explicit provider fails when used with module system #393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 38 additions & 20 deletions slf4j-api/src/main/java/org/slf4j/LoggerFactory.java
View file
Open in desktop
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
View file
Open in desktop
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