Merge pull request #86 from scijava/tenacious-creation

ctrueden · ctrueden · commit 0a1e8e568f16 · 2014-06-17T16:44:21.000-05:00
Add a non-strict mode for context creation
diff --git a/src/main/java/org/scijava/Context.java b/src/main/java/org/scijava/Context.java
@@ -58,6 +58,17 @@
  */
 public class Context implements Disposable {
 
+	// -- Constants --
+
+	/**
+	 * System property indicating whether the context should fail fast when
+	 * is attempts to instantiate a required service which is invalid or missing.
+	 * If this property is set to "false" then the context creation will attempt
+	 * to continue even when a required service cannot be instantiated. Otherwise,
+	 * the constructor will throw an {@link IllegalArgumentException} in that situation.
+	 */
+	public static final String STRICT_PROPERTY = "scijava.context.strict";
+
 	// -- Fields --
 
 	/** Index of the application context's services. */
@@ -66,7 +77,11 @@ public class Context implements Disposable {
 	/** Master index of all plugins known to the application context. */
 	private final PluginIndex pluginIndex;
 
-	/** Creates a new SciJava application context with all available services. */
+	/**
+	 * Creates a new SciJava application context with all available services.
+	 * 
+	 * @see #Context(Collection, PluginIndex, boolean)
+	 */
 	public Context() {
 		this(false);
 	}
@@ -76,6 +91,7 @@ public Context() {
 	 * 
 	 * @param empty If true, the context will be empty; otherwise, it will be
 	 *          initialized with all available services.
+	 * @see #Context(Collection, PluginIndex, boolean)
 	 */
 	@SuppressWarnings("unchecked")
 	public Context(final boolean empty) {
@@ -102,13 +118,12 @@ public Context(final boolean empty) {
 	 * 
 	 * @param serviceClasses A list of types that implement the {@link Service}
 	 *          interface (e.g., {@code DisplayService.class}).
+	 * @see #Context(Collection, PluginIndex, boolean)
 	 * @throws ClassCastException If any of the given arguments do not implement
 	 *           the {@link Service} interface.
 	 */
-	@SuppressWarnings({ "rawtypes", "unchecked" })
-	public Context(final Class... serviceClasses) {
-		this(serviceClasses != null ? (Collection) Arrays.asList(serviceClasses)
-			: Arrays.asList(Service.class));
+	public Context(@SuppressWarnings("rawtypes") final Class... serviceClasses) {
+		this(serviceClassList(serviceClasses));
 	}
 
 	/**
@@ -117,34 +132,69 @@ public Context(final Class... serviceClasses) {
 	 * 
 	 * @param serviceClasses A collection of types that implement the
 	 *          {@link Service} interface (e.g., {@code DisplayService.class}).
+	 * @see #Context(Collection, PluginIndex, boolean)
 	 */
 	public Context(final Collection<Class<? extends Service>> serviceClasses) {
 		this(serviceClasses, null);
 	}
 
+	/**
+	 * Creates a new SciJava application context with the specified services (and
+	 * any required service dependencies).
+	 * 
+	 * @param serviceClasses A collection of types that implement the
+	 *          {@link Service} interface (e.g., {@code DisplayService.class}).
+	 * @param strict Whether context creation will fail fast when there is
+	 *          is an error instantiating a required service.
+	 * @see #Context(Collection, PluginIndex, boolean)
+	 */
+	public Context(final Collection<Class<? extends Service>> serviceClasses,
+		final boolean strict)
+	{
+		this(serviceClasses, null, strict);
+	}
+
 	/**
 	 * Creates a new SciJava application with the specified PluginIndex. This
 	 * allows a base set of available plugins to be defined, and is useful when
-	 * plugins that would not be returned by the PluginIndex's PluginFinder are
-	 * desired.
-	 * <p>
-	 * NB: the {@link PluginIndex#discover()} method may still be called, adding
-	 * additional plugins to this index. The mechanism of discovery should be
-	 * configured exclusively through the attached
-	 * {@link org.scijava.plugin.PluginFinder}.
-	 * </p>
+	 * plugins that would not be returned by the {@link PluginIndex}'s
+	 * {@link org.scijava.plugin.PluginFinder} are desired.
 	 * 
 	 * @param pluginIndex The plugin index to use when discovering and indexing
 	 *          plugins. If you wish to completely control how services are
 	 *          discovered (i.e., use your own
 	 *          {@link org.scijava.plugin.PluginFinder} implementation), then you
-	 *          can pass a custom {@link PluginIndex} here.
+	 *          can pass a custom {@link PluginIndex} here. Passing null will
+	 *          result in a default plugin index being constructed and used.
+	 * @see #Context(Collection, PluginIndex, boolean)
 	 */
 	@SuppressWarnings("unchecked")
 	public Context(final PluginIndex pluginIndex) {
 		this(Arrays.<Class<? extends Service>> asList(Service.class), pluginIndex);
 	}
 
+	/**
+	 * Creates a new SciJava application context with the specified services (and
+	 * any required service dependencies). Service dependency candidates are
+	 * selected from those discovered by the given {@link PluginIndex}'s
+	 * associated {@link org.scijava.plugin.PluginFinder}.
+	 * 
+	 * @param serviceClasses A collection of types that implement the
+	 *          {@link Service} interface (e.g., {@code DisplayService.class}).
+	 * @param pluginIndex The plugin index to use when discovering and indexing
+	 *          plugins. If you wish to completely control how services are
+	 *          discovered (i.e., use your own
+	 *          {@link org.scijava.plugin.PluginFinder} implementation), then you
+	 *          can pass a custom {@link PluginIndex} here. Passing null will
+	 *          result in a default plugin index being constructed and used.
+	 * @see #Context(Collection, PluginIndex, boolean)
+	 */
+	public Context(final Collection<Class<? extends Service>> serviceClasses,
+		final PluginIndex pluginIndex)
+	{
+		this(serviceClasses, pluginIndex, strict());
+	}
+
 	/**
 	 * Creates a new SciJava application context with the specified services (and
 	 * any required service dependencies). Service dependency candidates are
@@ -166,17 +216,21 @@ public Context(final PluginIndex pluginIndex) {
 	 *          plugins. If you wish to completely control how services are
 	 *          discovered (i.e., use your own
 	 *          {@link org.scijava.plugin.PluginFinder} implementation), then you
-	 *          can pass a custom {@link PluginIndex} here.
+	 *          can pass a custom {@link PluginIndex} here. Passing null will
+	 *          result in a default plugin index being constructed and used.
+	 * @param strict Whether context creation will fail fast when there is
+	 *          is an error instantiating a required service.
 	 */
 	public Context(final Collection<Class<? extends Service>> serviceClasses,
-		final PluginIndex pluginIndex)
+		final PluginIndex pluginIndex, final boolean strict)
 	{
 		serviceIndex = new ServiceIndex();
 
 		this.pluginIndex = pluginIndex == null ? new PluginIndex() : pluginIndex;
 		this.pluginIndex.discover();
 
-		final ServiceHelper serviceHelper = new ServiceHelper(this, serviceClasses);
+		final ServiceHelper serviceHelper =
+			new ServiceHelper(this, serviceClasses, strict);
 		serviceHelper.loadServices();
 	}
 
@@ -323,6 +377,20 @@ public void dispose() {
 		}
 	}
 
+	// -- Utility methods --
+
+	/**
+	 * Utility method for converting a varargs list of service classes to a
+	 * {@link List} of those classes.
+	 */
+	@SuppressWarnings({ "rawtypes", "unchecked" })
+	public static List<Class<? extends Service>> serviceClassList(
+		final Class... serviceClasses)
+	{
+		return serviceClasses != null ? (List) Arrays.asList(serviceClasses)
+			: Arrays.asList(Service.class);
+	}
+
 	// -- Helper methods --
 
 	private String createMissingServiceMessage(
@@ -357,4 +425,8 @@ private String createMissingServiceMessage(
 		return msg.toString();
 	}
 
+	private static boolean strict() {
+		return !"false".equals(System.getProperty(STRICT_PROPERTY));
+	}
+
 }
diff --git a/src/main/java/org/scijava/service/ServiceHelper.java b/src/main/java/org/scijava/service/ServiceHelper.java
@@ -71,6 +71,12 @@ public class ServiceHelper extends AbstractContextual {
 	/** Classes to instantiate as services. */
 	private final List<Class<? extends Service>> serviceClasses;
 
+	/**
+	 * Whether service loading will fail fast when there is an error instantiating
+	 * a required service.
+	 */
+	private final boolean strict;
+
 	/**
 	 * Creates a new service helper for discovering and instantiating services.
 	 * 
@@ -89,6 +95,21 @@ public ServiceHelper(final Context context) {
 	 */
 	public ServiceHelper(final Context context,
 		final Collection<Class<? extends Service>> serviceClasses)
+	{
+		this(context, serviceClasses, true);
+	}
+
+	/**
+	 * Creates a new service helper for discovering and instantiating services.
+	 * 
+	 * @param context The application context to which services should be added.
+	 * @param serviceClasses The service classes to instantiate.
+	 * @param strict Whether service loading will fail fast when there is an error
+	 *          instantiating a required service.
+	 */
+	public ServiceHelper(final Context context,
+		final Collection<Class<? extends Service>> serviceClasses,
+		final boolean strict)
 	{
 		setContext(context);
 		log = context.getService(LogService.class);
@@ -108,6 +129,7 @@ public ServiceHelper(final Context context,
 			// load only the services that were explicitly specified
 			this.serviceClasses.addAll(serviceClasses);
 		}
+		this.strict = strict;
 	}
 
 	// -- ServiceHelper methods --
@@ -207,15 +229,19 @@ private <S extends Service> S loadService(final Class<S> c,
 				@SuppressWarnings("unchecked")
 				final S result = (S) createExactService(serviceClass, required);
 				if (required && result == null) {
-					throw new IllegalArgumentException();
+					final String error = "No match: " + serviceClass.getName();
+					if (strict) throw new IllegalArgumentException(error);
+					log.error(error);
 				}
 				return result;
 			}
 		}
 
 		if (required && c.isInterface()) {
-			throw new IllegalArgumentException("No compatible service: " +
-				c.getName());
+			final String error = "No compatible service: " + c.getName();
+			if (strict) throw new IllegalArgumentException(error);
+			log.error(error);
+			return null;
 		}
 
 		return createExactService(c, required);
@@ -249,9 +275,11 @@ private <S extends Service> S createExactService(final Class<S> c,
 		}
 		catch (final Throwable t) {
 			if (required) {
-				throw new IllegalArgumentException("Invalid service: " + name, t);
+				final String error = "Invalid service: " + name;
+				if (strict) throw new IllegalArgumentException(error, t);
+				log.error(error, t);
 			}
-			if (log.isDebug()) {
+			else if (log.isDebug()) {
 				// when in debug mode, give full stack trace of invalid services
 				log.debug("Invalid service: " + name, t);
 			}
@@ -290,8 +318,11 @@ private <S extends Service> S createServiceRecursively(final Class<S> c)
 				continue;
 			}
 			if (!Service.class.isAssignableFrom(type)) {
-				throw new IllegalArgumentException("Invalid parameter: " +
-					f.getDeclaringClass().getName() + "#" + f.getName());
+				final String error = "Invalid parameter: " +
+					f.getDeclaringClass().getName() + "#" + f.getName();
+				if (strict) throw new IllegalArgumentException(error);
+				log.error(error);
+				continue;
 			}
 			@SuppressWarnings("unchecked")
 			final Class<? extends Service> serviceType =
diff --git a/src/test/java/org/scijava/ContextCreationTest.java b/src/test/java/org/scijava/ContextCreationTest.java
@@ -39,6 +39,7 @@
 import static org.junit.Assert.fail;
 
 import java.util.Arrays;
+import java.util.List;
 
 import org.junit.Test;
 import org.scijava.plugin.Parameter;
@@ -190,6 +191,55 @@ public void testOptionalMissingTransitive() {
 		}
 	}
 
+	/**
+	 * Tests that missing {@link Service}s are handled properly in non-strict
+	 * mode; specifically, that {@link IllegalArgumentException} is <em>not</em>
+	 * thrown when attempting to create a {@link Context} requiring one directly.
+	 */
+	@Test
+	public void testNonStrictMissingDirect() {
+		final List<Class<? extends Service>> serviceClasses =
+			Context.serviceClassList(MissingService.class);
+		final Context context = new Context(serviceClasses, false);
+		assertEquals(0, context.getServiceIndex().size());
+	}
+
+	/**
+	 * Tests that missing {@link Service}s are handled properly in non-strict
+	 * mode; specifically, that {@link IllegalArgumentException} is <em>not</em>
+	 * thrown when attempting to create a {@link Context} requiring one
+	 * transitively.
+	 */
+	@Test
+	public void testNonStrictMissingTransitive() {
+		final List<Class<? extends Service>> serviceClasses =
+			Context.serviceClassList(MissingService.class);
+		final Context context = new Context(serviceClasses, false);
+		assertEquals(0, context.getServiceIndex().size());
+	}
+
+	/**
+	 * Tests that missing-but-optional {@link Service}s are handled properly in
+	 * non-strict mode; specifically, that {@link IllegalArgumentException} is
+	 * <em>not</em> thrown when attempting to create a {@link Context} requiring
+	 * one transitively.
+	 * <p>
+	 * A service marked {@link Optional}, but annotated without
+	 * {@code required = false} from a dependent service, is assumed to be
+	 * required for that dependent service.
+	 * </p>
+	 */
+	@Test
+	public void testNonStrictOptionalMissingTransitive() {
+		final List<Class<? extends Service>> serviceClasses =
+			Context.serviceClassList(ServiceRequiringOptionalMissingService.class);
+		final Context context = new Context(serviceClasses, false);
+		final List<Service> services = context.getServiceIndex().getAll();
+		assertEquals(1, services.size());
+		assertSame(ServiceRequiringOptionalMissingService.class, services.get(0)
+			.getClass());
+	}
+
 	/**
 	 * Verifies that the order plugins appear in the PluginIndex and Service list
 	 * does not affect which services are loaded.
