Services now checks the contextClassLoader, Services.class.classLoader, and the system classloader

Fixes: #568

CHANGELOG.md

@@ -6,6 +6,7 @@ This patch release:
 
 * Fixes issue when using Java 9+ `Map.of` with JacksonDeserializer which resulted in an NullPointerException
 * Fixes issue preventing Gson seralizer/deserializer implementation from being detected automatically
+* Services are now loaded from the context class loader, Services.class.classLoader, and the system classloader, the first classloader with a service wins, and the others are ignored. This mimics how `Classes.forName()` works, and how JJWT attempted to auto-discover various implementations in previous versions.
 
 ### 0.11.0
 

api/src/main/java/io/jsonwebtoken/lang/Classes.java

@@ -85,8 +85,8 @@ public final class Classes {
             String msg = "Unable to load class named [" + fqcn + "] from the thread context, current, or " +
                     "system/application ClassLoaders.  All heuristics have been exhausted.  Class could not be found.";
 
-            if (fqcn != null && fqcn.startsWith("com.stormpath.sdk.impl")) {
-                msg += "  Have you remembered to include the stormpath-sdk-impl .jar in your runtime classpath?";
+            if (fqcn != null && fqcn.startsWith("io.jsonwebtoken.impl")) {
+                msg += "  Have you remembered to include the jjwt-impl.jar in your runtime classpath?";
             }
 
             throw new UnknownClassException(msg);

impl/src/main/java/io/jsonwebtoken/impl/lang/Services.java

@@ -22,12 +22,35 @@ import java.util.Collections;
 import java.util.List;
 import java.util.ServiceLoader;
 
+import static io.jsonwebtoken.lang.Collections.arrayToList;
+
 /**
  * Helper class for loading services from the classpath, using a {@link ServiceLoader}. Decouples loading logic for
  * better separation of concerns and testability.
  */
 public final class Services {
 
+    private static final List<ClassLoaderAccessor> CLASS_LOADER_ACCESSORS = arrayToList(new ClassLoaderAccessor[] {
+            new ClassLoaderAccessor() {
+                @Override
+                public ClassLoader getClassLoader() {
+                    return Thread.currentThread().getContextClassLoader();
+                }
+            },
+            new ClassLoaderAccessor() {
+                @Override
+                public ClassLoader getClassLoader() {
+                    return Services.class.getClassLoader();
+                }
+            },
+            new ClassLoaderAccessor() {
+                @Override
+                public ClassLoader getClassLoader() {
+                    return ClassLoader.getSystemClassLoader();
+                }
+            }
+    });
+
     private Services() {}
 
     /**
@@ -40,20 +63,24 @@ public final class Services {
      */
     public static <T> List<T> loadAll(Class<T> spi) {
         Assert.notNull(spi, "Parameter 'spi' must not be null.");
-        ServiceLoader<T> serviceLoader = ServiceLoader.load(spi);
 
-        List<T> implementations = new ArrayList<>();
-
-        for (T implementation : serviceLoader) {
-            implementations.add(implementation);
+        for (ClassLoaderAccessor classLoaderAccessor : CLASS_LOADER_ACCESSORS) {
+            List<T> implementations = loadAll(spi, classLoaderAccessor.getClassLoader());
+            if (!implementations.isEmpty()) {
+                return Collections.unmodifiableList(implementations);
+            }
         }
 
-        // fail if no implementations were found
-        if (implementations.isEmpty()) {
         throw new UnavailableImplementationException(spi);
     }
 
-        return Collections.unmodifiableList(implementations);
+    private static <T> List<T> loadAll(Class<T> spi, ClassLoader classLoader) {
+        ServiceLoader<T> serviceLoader = ServiceLoader.load(spi, classLoader);
+        List<T> implementations = new ArrayList<>();
+        for (T implementation : serviceLoader) {
+            implementations.add(implementation);
+        }
+        return implementations;
     }
 
     /**
@@ -68,11 +95,25 @@ public final class Services {
      */
     public static <T> T loadFirst(Class<T> spi) {
         Assert.notNull(spi, "Parameter 'spi' must not be null.");
-        ServiceLoader<T> serviceLoader = ServiceLoader.load(spi);
-        if (serviceLoader.iterator().hasNext()) {
-            return serviceLoader.iterator().next();
-        } else {
+
+        for (ClassLoaderAccessor classLoaderAccessor : CLASS_LOADER_ACCESSORS) {
+            T result = loadFirst(spi, classLoaderAccessor.getClassLoader());
+            if (result != null) {
+                return result;
+            }
+        }
         throw new UnavailableImplementationException(spi);
     }
+
+    private static <T> T loadFirst(Class<T> spi, ClassLoader classLoader) {
+        ServiceLoader<T> serviceLoader = ServiceLoader.load(spi, classLoader);
+        if (serviceLoader.iterator().hasNext()) {
+            return serviceLoader.iterator().next();
+        }
+        return null;
+    }
+
+    private interface ClassLoaderAccessor {
+        ClassLoader getClassLoader();
     }
 }

impl/src/test/groovy/io/jsonwebtoken/impl/lang/ServicesTest.groovy

@@ -19,9 +19,12 @@ import io.jsonwebtoken.impl.DefaultStubService
 import io.jsonwebtoken.StubService
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.powermock.api.easymock.PowerMock
 import org.powermock.core.classloader.annotations.PrepareForTest
 import org.powermock.modules.junit4.PowerMockRunner
 
+import java.lang.reflect.Field
+
 import static org.junit.Assert.assertEquals
 import static org.junit.Assert.assertNotNull
 
@@ -55,6 +58,15 @@ class ServicesTest {
         new Services(); // not allowed in Java, including here for test coverage
     }
 
+    @Test
+    void testClassLoaderAccessorList() {
+        List<Services.ClassLoaderAccessor> accessorList = Services.CLASS_LOADER_ACCESSORS
+        assertEquals("Expected 3 ClassLoaderAccessor to be found", 3, accessorList.size())
+        assertEquals(Thread.currentThread().getContextClassLoader(), accessorList.get(0).getClassLoader())
+        assertEquals(Services.class.getClassLoader(), accessorList.get(1).getClassLoader())
+        assertEquals(ClassLoader.getSystemClassLoader(), accessorList.get(2).getClassLoader())
+    }
+
     static class NoServicesClassLoader extends ClassLoader {
         private NoServicesClassLoader(ClassLoader parent) {
             super(parent)
@@ -70,14 +82,22 @@ class ServicesTest {
         }
 
         static void runWith(Closure closure) {
-            ClassLoader originalClassloader = Thread.currentThread().getContextClassLoader()
+            Field field = PowerMock.field(Services.class, "CLASS_LOADER_ACCESSORS")
+            def originalValue = field.get(Services.class)
             try {
-                Thread.currentThread().setContextClassLoader(new NoServicesClassLoader(originalClassloader))
+                // use powermock to change the list of the classloaders we are using
+                List<Services.ClassLoaderAccessor> classLoaderAccessors = [
+                        new Services.ClassLoaderAccessor() {
+                            @Override
+                            ClassLoader getClassLoader() {
+                                return new NoServicesClassLoader(Thread.currentThread().getContextClassLoader())
+                            }
+                        }
+                ]
+                field.set(Services.class, classLoaderAccessors)
                 closure.run()
             } finally {
-                if (originalClassloader != null) {
-                    Thread.currentThread().setContextClassLoader(originalClassloader)
-                }
+                field.set(Services.class, originalValue)
             }
         }
     }