Merge branch 'command-context-injection'

This branch fixes a serious flaw in how Service and Context
parameters of Commands were being handled. It also provides a unit
test for the new parameter validater function mechanism, which
doubles as a test for the fixed Service parameter injection behavior.

This branch is dedicated to Richard Domander.

Author: ctrueden

src/main/java/org/scijava/command/CommandInfo.java

@@ -41,7 +41,6 @@
 import java.util.Map;
 
 import org.scijava.Cancelable;
-import org.scijava.Context;
 import org.scijava.InstantiableException;
 import org.scijava.ItemIO;
 import org.scijava.ItemVisibility;
@@ -55,7 +54,6 @@
 import org.scijava.plugin.Parameter;
 import org.scijava.plugin.Plugin;
 import org.scijava.plugin.PluginInfo;
-import org.scijava.service.Service;
 import org.scijava.util.ClassUtils;
 import org.scijava.util.StringMaker;
 
@@ -451,11 +449,6 @@ private void checkFields(final Class<?> type) {
 		for (final Field f : fields) {
 			f.setAccessible(true); // expose private fields
 
-			// NB: Skip types handled by the application framework itself.
-			// I.e., these parameters get injected by Context#inject(Object).
-			if (Service.class.isAssignableFrom(f.getType())) continue;
-			if (Context.class.isAssignableFrom(f.getType())) continue;
-
 			final Parameter param = f.getAnnotation(Parameter.class);
 
 			boolean valid = true;

src/main/java/org/scijava/command/CommandModule.java

@@ -39,7 +39,6 @@
 import org.scijava.InstantiableException;
 import org.scijava.NullContextException;
 import org.scijava.module.AbstractModule;
-import org.scijava.module.MethodCallException;
 import org.scijava.module.Module;
 import org.scijava.module.ModuleException;
 import org.scijava.module.ModuleInfo;
@@ -144,13 +143,6 @@ public void cancel() {
 		previewPlugin.cancel();
 	}
 
-	@Override
-	public void initialize() throws MethodCallException {
-		// NB: Inject the context into the command before initializing.
-		getContext().inject(command);
-		super.initialize();
-	}
-
 	@Override
 	public CommandInfo getInfo() {
 		return info;

src/main/java/org/scijava/module/process/GatewayPreprocessor.java

@@ -52,8 +52,8 @@
  * 
  * @author Curtis Rueden
  */
-@Plugin(type = PreprocessorPlugin.class,
-	priority = Priority.VERY_HIGH_PRIORITY)
+@Plugin(type = PreprocessorPlugin.class, //
+	priority = 2 * Priority.VERY_HIGH_PRIORITY)
 public class GatewayPreprocessor extends AbstractPreprocessorPlugin {
 
 	@Parameter
@@ -87,22 +87,22 @@ private <G extends Gateway> void setGatewayValue(final Context context,
 		try {
 			gateway = type.getConstructor(Context.class).newInstance(context);
 		}
-		catch (IllegalArgumentException exc) {
+		catch (final IllegalArgumentException exc) {
 			exception = exc;
 		}
-		catch (SecurityException exc) {
+		catch (final SecurityException exc) {
 			exception = exc;
 		}
-		catch (InstantiationException exc) {
+		catch (final InstantiationException exc) {
 			exception = exc;
 		}
-		catch (IllegalAccessException exc) {
+		catch (final IllegalAccessException exc) {
 			exception = exc;
 		}
-		catch (InvocationTargetException exc) {
+		catch (final InvocationTargetException exc) {
 			exception = exc;
 		}
-		catch (NoSuchMethodException exc) {
+		catch (final NoSuchMethodException exc) {
 			exception = exc;
 		}
 		if (exception != null) {

src/main/java/org/scijava/module/process/SaveInputsPreprocessor.java

@@ -43,8 +43,8 @@
  * <p>
  * This preprocessor runs late in the chain, giving other preprocessors every
  * chance to populate the inputs first. In particular, it executes after the
- * {@link org.scijava.widget.InputHarvester} has run, so that user-specified values
- * are persisted for next time.
+ * {@link org.scijava.widget.InputHarvester} has run, so that user-specified
+ * values are persisted for next time.
  * </p>
  * 
  * @author Curtis Rueden

src/main/java/org/scijava/module/process/ServicePreprocessor.java

@@ -61,8 +61,8 @@
  * 
  * @author Curtis Rueden
  */
-@Plugin(type = PreprocessorPlugin.class,
-	priority = Priority.VERY_HIGH_PRIORITY)
+@Plugin(type = PreprocessorPlugin.class, //
+	priority = 2 * Priority.VERY_HIGH_PRIORITY)
 public class ServicePreprocessor extends AbstractPreprocessorPlugin {
 
 	// -- ModuleProcessor methods --

src/main/java/org/scijava/module/process/ValidityPreprocessor.java

@@ -44,7 +44,7 @@
  * @author Curtis Rueden
  */
 @Plugin(type = PreprocessorPlugin.class,
-	priority = Priority.VERY_HIGH_PRIORITY + 1)
+	priority = 3 * Priority.VERY_HIGH_PRIORITY)
 public class ValidityPreprocessor extends AbstractPreprocessorPlugin {
 
 	// -- ModuleProcessor methods --

src/test/java/org/scijava/command/CommandModuleTest.java

@@ -33,18 +33,24 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.util.concurrent.ExecutionException;
 
 import org.junit.Test;
 import org.scijava.Cancelable;
 import org.scijava.Context;
+import org.scijava.ItemIO;
+import org.scijava.Priority;
+import org.scijava.log.LogService;
 import org.scijava.module.Module;
+import org.scijava.module.ModuleItem;
 import org.scijava.module.process.AbstractPreprocessorPlugin;
 import org.scijava.module.process.PreprocessorPlugin;
 import org.scijava.plugin.Parameter;
 import org.scijava.plugin.Plugin;
+import org.scijava.service.Service;
 
 /** Regression tests for {@link CommandModule}. */
 public class CommandModuleTest {
@@ -64,7 +70,6 @@ public void testCancelable() throws InterruptedException, ExecutionException {
 		assertFalse(crow.isCanceled());
 	}
 
-
 	@Test
 	public void testNotCancelable() throws InterruptedException,
 		ExecutionException
@@ -96,6 +101,17 @@ public void testDefaultValues() {
 		assertEquals(null, info.getInput("thing").getDefaultValue());
 	}
 
+	@Test
+	public void testValidation() throws InterruptedException, ExecutionException {
+		final Context context = new Context(CommandService.class);
+		final CommandService commandService = context.service(CommandService.class);
+
+		final CommandModule module = //
+			commandService.run(CommandWithValidation.class, true).get();
+		assertNotNull(module.getInput("stuff"));
+		assertEquals("success", module.getOutput("result"));
+	}
+
 	// -- Helper classes --
 
 	/** A command which implements {@link Cancelable}. */
@@ -168,4 +184,55 @@ public void run() {
 			time = 0;
 		}
 	}
+
+	/** A command which validates an input. */
+	@Plugin(type = Command.class)
+	public static class CommandWithValidation extends ContextCommand {
+
+		@Parameter
+		private LogService log;
+
+		@Parameter(validater = "validateStuff")
+		private Stuff stuff;
+
+		@Parameter(type = ItemIO.OUTPUT)
+		private String result = "default";
+
+		@SuppressWarnings("unused")
+		private void validateStuff() {
+			final StringBuilder sb = new StringBuilder();
+			if (log == null) sb.append("[null-log] ");
+			if (stuff == null) sb.append("[null-stuff] ");
+			result = sb.length() == 0 ? "success" : sb.toString();
+		}
+
+		@Override
+		public void run() {
+			if (!result.equals("success")) result += " failure";
+		}
+	}
+
+	/**
+	 * Preprocessor to inject {@link Stuff} instances very early. But (in theory)
+	 * not as early as {@link Service} and {@link Context} parameters get
+	 * populated.
+	 */
+	@Plugin(type = PreprocessorPlugin.class,
+		priority = Priority.VERY_HIGH_PRIORITY)
+	public static class StuffPreprocessor extends AbstractPreprocessorPlugin {
+
+		@Override
+		public void process(final Module module) {
+			for (final ModuleItem<?> input : module.getInfo().inputs()) {
+				if (Stuff.class.isAssignableFrom(input.getType())) {
+					module.setInput(input.getName(), new Stuff());
+					module.resolveInput(input.getName());
+				}
+			}
+		}
+
+	}
+
+	/** Placeholder class, for type safety. */
+	public static class Stuff {}
 }