Merge pull request #27 from cryptomator/feature/26-robust-service-loading

Feature:  Robust service provider loading

Author: infeo

src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java

@@ -10,6 +10,7 @@
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.Optional;
+import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
 import java.util.stream.Stream;
 
@@ -47,7 +48,7 @@ public static <T> Stream<T> loadAll(Class<T> clazz) {
 				.filter(IntegrationsLoader::isSupportedOperatingSystem)
 				.filter(IntegrationsLoader::passesStaticAvailabilityCheck)
 				.sorted(Comparator.comparingInt(IntegrationsLoader::getPriority).reversed())
-				.map(ServiceLoader.Provider::get)
+				.flatMap(IntegrationsLoader::instantiateServiceProvider)
 				.filter(IntegrationsLoader::passesInstanceAvailabilityCheck)
 				.peek(impl -> logServiceIsAvailable(clazz, impl.getClass()));
 	}
@@ -68,18 +69,30 @@ private static boolean isSupportedOperatingSystem(ServiceLoader.Provider<?> prov
 		return annotations.length == 0 || Arrays.stream(annotations).anyMatch(OperatingSystem.Value::isCurrent);
 	}
 
+	private static <T> Stream<T> instantiateServiceProvider(ServiceLoader.Provider<T> provider) {
+		try {
+			return Stream.of(provider.get());
+		} catch (ServiceConfigurationError err) {
+			//ServiceLoader.Provider::get throws this error if (from javadoc)
+			// * the public static "provider()" method of a provider factory returns null
+			// * the service provider cannot be instantiated due to an error/throw
+			LOG.warn("Unable to load service provider {}.", provider.type().getName(), err);
+			return Stream.empty();
+		}
+	}
+
 	private static boolean passesStaticAvailabilityCheck(ServiceLoader.Provider<?> provider) {
 		return passesStaticAvailabilityCheck(provider.type());
 	}
 
 	@VisibleForTesting
 	static boolean passesStaticAvailabilityCheck(Class<?> type) {
-		return passesAvailabilityCheck(type, null);
+		return silentlyPassesAvailabilityCheck(type, null);
 	}
 
 	@VisibleForTesting
 	static boolean passesInstanceAvailabilityCheck(Object instance) {
-		return passesAvailabilityCheck(instance.getClass(), instance);
+		return silentlyPassesAvailabilityCheck(instance.getClass(), instance);
 	}
 
 	private static void logServiceIsAvailable(Class<?> apiType, Class<?> implType) {
@@ -88,6 +101,15 @@ private static void logServiceIsAvailable(Class<?> apiType, Class<?> implType) {
 		}
 	}
 
+	private static <T> boolean silentlyPassesAvailabilityCheck(Class<? extends T> type, @Nullable T instance) {
+		try {
+			return passesAvailabilityCheck(type, instance);
+		} catch (ExceptionInInitializerError | NoClassDefFoundError | RuntimeException e) {
+			LOG.warn("Unable to load service provider {}.", type.getName(), e);
+			return false;
+		}
+	}
+
 	private static <T> boolean passesAvailabilityCheck(Class<? extends T> type, @Nullable T instance) {
 		if (!type.isAnnotationPresent(CheckAvailability.class)) {
 			return true; // if type is not annotated, skip tests

src/test/java/org/cryptomator/integrations/common/InitExceptionTestClass.java

@@ -0,0 +1,25 @@
+package org.cryptomator.integrations.common;
+
+@CheckAvailability
+public class InitExceptionTestClass {
+
+	private static final String TEST;
+
+	static {
+		TEST = throwSomething();
+	}
+
+	public InitExceptionTestClass() {
+
+	}
+
+	static String throwSomething() {
+		throw new RuntimeException("STATIC FAIL");
+	}
+
+	@CheckAvailability
+	public static boolean test() {
+		return true;
+	}
+
+}

src/test/java/org/cryptomator/integrations/common/IntegrationsLoaderTest.java

@@ -97,6 +97,18 @@ class C2 extends StaticFalse {
 			Assertions.assertFalse(IntegrationsLoader.passesStaticAvailabilityCheck(C3.class));
 		}
 
+		@Test
+		@DisplayName("throwing @CheckAvailability methods are treated as false")
+		public void testPassesAvailabilityCheckThrowing() {
+
+			@CheckAvailability class C1 {
+				@CheckAvailability public static boolean test() { throw new RuntimeException("FAIL"); }
+			}
+
+			Assertions.assertFalse(IntegrationsLoader.passesStaticAvailabilityCheck(C1.class));
+			Assertions.assertFalse(IntegrationsLoader.passesStaticAvailabilityCheck(InitExceptionTestClass.class));
+			Assertions.assertFalse(IntegrationsLoader.passesStaticAvailabilityCheck(InitExceptionTestClass.class)); //NoClassDefFoundError due to repated call
+		}
 
 	}
 
@@ -190,6 +202,26 @@ class C2 extends InstanceFalse {
 			Assertions.assertFalse(IntegrationsLoader.passesInstanceAvailabilityCheck(new C3()));
 		}
 
+
+		@Test
+		@DisplayName("throwing @CheckAvailability methods are treated as false")
+		public void testPassesAvailabilityCheckThrowing() {
+
+			@CheckAvailability
+			class C1 {
+				@CheckAvailability public boolean test1() { throw new RuntimeException("FAIL"); }
+			}
+
+			@CheckAvailability
+			class C2 {
+				@CheckAvailability public boolean test1() { return true; }
+				@CheckAvailability public boolean test2() { throw new RuntimeException("FAIL"); }
+			}
+
+			Assertions.assertFalse(IntegrationsLoader.passesInstanceAvailabilityCheck(new C1()));
+			Assertions.assertFalse(IntegrationsLoader.passesInstanceAvailabilityCheck(new C2()));
+		}
+
 	}
 
 }