Avoid calling chooseFiles for non-autofill inputs

This commit changes the API of ModuleService and DefaultModuleService to include an optional parameter 'acrossTypes' that allows checking for solitary unresolved parameters within all inputs/outputs independent of their type, while maintaining the backwards compatible method signatures.

We can then replace some duplicated logic in helper methods of FilePreprocessor and FileListPreprocessor by a call to 'moduleService.getSingleInput()', which makes them respect an 'autoFill=false' parameter annotation and avoid calling uiService.chooseFile/chooseFiles for those parameters.

The intended behavior is verified by two new tests that, before this commit, are failing on their last assertion.

Author: imagejan

src/main/java/org/scijava/module/DefaultModuleService.java

@@ -260,27 +260,23 @@ public <M extends Module> M waitFor(final Future<M> future) {
 	}
 
 	@Override
-	public <T> ModuleItem<T> getSingleInput(final Module module,
-		final Class<T> type)
-	{
-		return getTypedSingleItem(module, type, module.getInfo().inputs());
+	public <T> ModuleItem<T> getSingleInput(final Module module, final Class<T> type, boolean acrossTypes) {
+		return getTypedSingleItem(module, type, module.getInfo().inputs(), acrossTypes);
 	}
 
 	@Override
-	public <T> ModuleItem<T> getSingleOutput(final Module module,
-		final Class<T> type)
-	{
-		return getTypedSingleItem(module, type, module.getInfo().outputs());
+	public <T> ModuleItem<T> getSingleOutput(final Module module, final Class<T> type, boolean acrossTypes) {
+		return getTypedSingleItem(module, type, module.getInfo().outputs(), acrossTypes);
 	}
 
 	@Override
-	public ModuleItem<?> getSingleInput(Module module, Collection<Class<?>> types) {
-		return getSingleItem(module, types, module.getInfo().inputs());
+	public ModuleItem<?> getSingleInput(Module module, Collection<Class<?>> types, boolean acrossTypes) {
+		return getSingleItem(module, types, module.getInfo().inputs(), acrossTypes);
 	}
 
 	@Override
-	public ModuleItem<?> getSingleOutput(Module module, Collection<Class<?>> types) {
-		return getSingleItem(module, types, module.getInfo().outputs());
+	public ModuleItem<?> getSingleOutput(Module module, Collection<Class<?>> types, boolean acrossTypes) {
+		return getSingleItem(module, types, module.getInfo().outputs(), acrossTypes);
 	}
 
 	@Override
@@ -478,24 +474,25 @@ private void assignInputs(final Module module,
 	}
 
 	private <T> ModuleItem<T> getTypedSingleItem(final Module module,
-		final Class<T> type, final Iterable<ModuleItem<?>> items)
+		final Class<T> type, final Iterable<ModuleItem<?>> items, boolean acrossTypes)
 	{
 		Set<Class<?>> types = new HashSet<>();
 		types.add(type);
 		@SuppressWarnings("unchecked")
-		ModuleItem<T> result = (ModuleItem<T>) getSingleItem(module, types, items);
+		ModuleItem<T> result = (ModuleItem<T>) getSingleItem(module, types, items, acrossTypes);
 		return result;
 	}
 
 	private ModuleItem<?> getSingleItem(final Module module,
-		final Collection<Class<?>> types, final Iterable<ModuleItem<?>> items)
+		final Collection<Class<?>> types, final Iterable<ModuleItem<?>> items, boolean acrossTypes)
 	{
 		ModuleItem<?> result = null;
 
 		for (final ModuleItem<?> item : items) {
 			final String name = item.getName();
 			if (!item.isAutoFill()) continue; // skip unfillable inputs
 			if (module.isInputResolved(name)) continue; // skip resolved inputs
+			if (acrossTypes && result != null) return null; // multiple unresolved items
 			final Class<?> itemType = item.getType();
 			for (final Class<?> type : types) {
 				if (type.isAssignableFrom(itemType)) {

src/main/java/org/scijava/module/ModuleService.java

@@ -272,26 +272,66 @@ <M extends Module> Future<M> run(M module,
 	 * given type, returning the relevant {@link ModuleItem} if found, or null if
 	 * not exactly one unresolved fillable input of that type.
 	 */
-	<T> ModuleItem<T> getSingleInput(Module module, Class<T> type);
+	default <T> ModuleItem<T> getSingleInput(Module module, Class<T> type) {
+		return getSingleInput(module, type, false);
+	}
+
+	/**
+	 * Checks the given module for a solitary unresolved fillable input of the
+	 * given type, returning the relevant {@link ModuleItem} if found, or null if
+	 * not exactly one unresolved fillable input.
+	 * 
+	 * If {@code acrossTpyes} is true, all inputs independent of type are taken
+	 * into account when checking for singularity of the unresolved input. 
+	 */
+	<T> ModuleItem<T> getSingleInput(Module module, Class<T> type, boolean acrossTypes);
 
 	/**
 	 * Checks the given module for a solitary unresolved output of the given type,
 	 * returning the relevant {@link ModuleItem} if found, or null if not exactly
 	 * one unresolved output of that type.
 	 */
-	<T> ModuleItem<T> getSingleOutput(Module module, Class<T> type);
+	default <T> ModuleItem<T> getSingleOutput(Module module, Class<T> type) {
+		return getSingleOutput(module, type, false);
+	}
+
+	/**
+	 * Checks the given module for a solitary unresolved output of the given type,
+	 * returning the relevant {@link ModuleItem} if found, or null if not exactly
+	 * one unresolved output.
+	 * 
+	 * If {@code acrossTpyes} is true, all outputs independent of type are taken
+	 * into account when checking for singularity of the unresolved output. 
+	 */
+	<T> ModuleItem<T> getSingleOutput(Module module, Class<T> type, boolean acrossTypes);
 
 	/**
 	 * As {@link #getSingleInput(Module, Class)} but will match with a set of
 	 * potential classes, at the cost of generic parameter safety.
 	 */
-	ModuleItem<?> getSingleInput(Module module, Collection<Class<?>> types);
+	default ModuleItem<?> getSingleInput(Module module, Collection<Class<?>> types) {
+		return getSingleInput(module, types, false);
+	}
+
+	/**
+	 * As {@link #getSingleInput(Module, Class, boolean)} but will match with a set of
+	 * potential classes, at the cost of generic parameter safety.
+	 */
+	ModuleItem<?> getSingleInput(Module module, Collection<Class<?>> types, boolean acrossTypes);
 
 	/**
 	 * As {@link #getSingleOutput(Module, Class)} but will match with a set of
 	 * potential classes, at the cost of generic parameter safety.
 	 */
-	ModuleItem<?> getSingleOutput(Module module, Collection<Class<?>> types);
+	default ModuleItem<?> getSingleOutput(Module module, Collection<Class<?>> types) {
+		return getSingleOutput(module, types, false);
+	}
+
+	/**
+	 * As {@link #getSingleOutput(Module, Class, boolean)} but will match with a set of
+	 * potential classes, at the cost of generic parameter safety.
+	 */
+	ModuleItem<?> getSingleOutput(Module module, Collection<Class<?>> types, boolean acrossTypes);
 
 	/**
 	 * Registers the given value for the given {@link ModuleItem} using the

src/main/java/org/scijava/ui/FileListPreprocessor.java

@@ -32,6 +32,7 @@
 
 import org.scijava.module.Module;
 import org.scijava.module.ModuleItem;
+import org.scijava.module.ModuleService;
 import org.scijava.module.process.AbstractPreprocessorPlugin;
 import org.scijava.module.process.PreprocessorPlugin;
 import org.scijava.plugin.Parameter;
@@ -44,10 +45,13 @@ public class FileListPreprocessor extends AbstractPreprocessorPlugin {
 	@Parameter(required = false)
 	private UIService uiService;
 
+	@Parameter(required = false)
+	private ModuleService moduleService;
+
 	@Override
 	public void process(final Module module) {
-		if (uiService == null) return;
-		final ModuleItem<File[]> fileInput = getFilesInput(module);
+		if (uiService == null || moduleService == null) return;
+		final ModuleItem<File[]> fileInput = moduleService.getSingleInput(module, File[].class, true);
 		if (fileInput == null) return;
 
 		final File[] files = fileInput.getValue(module);
@@ -65,30 +69,4 @@ public void process(final Module module) {
 		module.resolveInput(fileInput.getName());
 	}
 
-	// -- Helper methods --
-
-	/**
-	 * Gets the single unresolved {@link File} input parameter. If there is not
-	 * exactly one unresolved {@link File} input parameter, or if there are other
-	 * types of unresolved parameters, this method returns null.
-	 */
-	private ModuleItem<File[]> getFilesInput(final Module module) {
-		ModuleItem<File[]> result = null;
-		for (final ModuleItem<?> input : module.getInfo().inputs()) {
-			if (module.isInputResolved(input.getName())) continue;
-			final Class<?> type = input.getType();
-			if (!File[].class.isAssignableFrom(type)) {
-				// not a File[] parameter; abort
-				return null;
-			}
-			if (result != null) {
-				// second File parameter; abort
-				return null;
-			}
-			@SuppressWarnings("unchecked")
-			final ModuleItem<File[]> fileInput = (ModuleItem<File[]>) input;
-			result = fileInput;
-		}
-		return result;
-	}
 }

src/main/java/org/scijava/ui/FilePreprocessor.java

@@ -33,6 +33,7 @@
 
 import org.scijava.module.Module;
 import org.scijava.module.ModuleItem;
+import org.scijava.module.ModuleService;
 import org.scijava.module.process.AbstractPreprocessorPlugin;
 import org.scijava.module.process.PreprocessorPlugin;
 import org.scijava.plugin.Parameter;
@@ -53,12 +54,15 @@ public class FilePreprocessor extends AbstractPreprocessorPlugin {
 	@Parameter(required = false)
 	private UIService uiService;
 
+	@Parameter(required = false)
+	private ModuleService moduleService;
+
 	// -- ModuleProcessor methods --
 
 	@Override
 	public void process(final Module module) {
-		if (uiService == null) return;
-		final ModuleItem<File> fileInput = getFileInput(module);
+		if (uiService == null || moduleService == null) return;
+		final ModuleItem<File> fileInput = moduleService.getSingleInput(module, File.class, true);
 		if (fileInput == null) return;
 
 		final File file = fileInput.getValue(module);
@@ -75,31 +79,4 @@ public void process(final Module module) {
 		module.resolveInput(fileInput.getName());
 	}
 
-	// -- Helper methods --
-
-	/**
-	 * Gets the single unresolved {@link File} input parameter. If there is not
-	 * exactly one unresolved {@link File} input parameter, or if there are other
-	 * types of unresolved parameters, this method returns null.
-	 */
-	private ModuleItem<File> getFileInput(final Module module) {
-		ModuleItem<File> result = null;
-		for (final ModuleItem<?> input : module.getInfo().inputs()) {
-			if (module.isInputResolved(input.getName())) continue;
-			final Class<?> type = input.getType();
-			if (!File.class.isAssignableFrom(type)) {
-				// not a File parameter; abort
-				return null;
-			}
-			if (result != null) {
-				// second File parameter; abort
-				return null;
-			}
-			@SuppressWarnings("unchecked")
-			final ModuleItem<File> fileInput = (ModuleItem<File>) input;
-			result = fileInput;
-		}
-		return result;
-	}
-
 }

src/test/java/org/scijava/ui/FileListPreprocessorTest.java

@@ -0,0 +1,109 @@
+package org.scijava.ui;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import java.io.File;
+import java.io.FileFilter;
+import java.util.concurrent.ExecutionException;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.scijava.Context;
+import org.scijava.command.Command;
+import org.scijava.command.CommandInfo;
+import org.scijava.module.Module;
+import org.scijava.module.ModuleService;
+import org.scijava.plugin.Parameter;
+import org.scijava.ui.headless.HeadlessUI;
+
+public class FileListPreprocessorTest {
+
+	public static final String DUMMY_FILE_NAME = "dummy_file";
+	private Context context;
+	private ModuleService moduleService;
+
+	@Before
+	public void setUp() {
+		context = new Context();
+		context.service(UIService.class).setDefaultUI(new CustomUI());
+		context.service(UIService.class).setHeadless(false);
+		moduleService = context.service(ModuleService.class);
+	}
+
+	@After
+	public void tearDown() {
+		context.dispose();
+	}
+
+	@Test
+	public void test() throws InterruptedException, ExecutionException {
+		// assert that single File[] parameters get filled by ui.chooseFiles()
+		CommandInfo info = new CommandInfo(SingleParameterFileListCommand.class);
+		Module mod = moduleService.run(info, true).get();
+		assertNotNull(mod);
+		assertEquals(DUMMY_FILE_NAME, ((File[])mod.getInput("inputFiles"))[0].getName());
+
+		// assert that the presence of any other unresolved parameters prevents
+		// calls to ui.chooseFiles()
+		CommandInfo info2 = new CommandInfo(ParameterFileListAndOthersCommand.class);
+		Module mod2 = moduleService.run(info2, true).get();
+		assertNotNull(mod2);
+		assertNull(mod2.getInput("inputFiles"));
+
+		// assert that 'autoFill = false' prevents calls to ui.chooseFiles()
+		CommandInfo info3 = new CommandInfo(NoAutofillParameterFileListCommand.class);
+		Module mod3 = moduleService.run(info3, true).get();
+		assertNotNull(mod3);
+		assertNull(mod3.getInput("inputFiles"));
+	}
+
+	private class CustomUI extends HeadlessUI {
+
+		@Override
+		public File[] chooseFiles(File parent, File[] files, FileFilter filter, String style) {
+			return new File[] { new File(DUMMY_FILE_NAME) };
+		}
+	}
+
+	public static class SingleParameterFileListCommand implements Command {
+
+		@Parameter(persist = false)
+		private File[] inputFiles;
+
+		@Override
+		public void run() {
+			// do nothing
+		}
+
+	}
+
+	public static class ParameterFileListAndOthersCommand implements Command {
+
+		@Parameter(persist = false)
+		private File[] inputFiles;
+
+		@Parameter(required = false)
+		private String dummyString;
+
+		@Override
+		public void run() {
+			// do nothing
+		}
+
+	}
+
+	public static class NoAutofillParameterFileListCommand implements Command {
+
+		@Parameter(autoFill = false, persist = false)
+		private File[] inputFiles;
+
+		@Override
+		public void run() {
+			// do nothing
+		}
+
+	}
+}