Merge branch 'smart-run-with-map'

This makes ModuleService#run smarter when given a map as the
sole argument to the "Object... input" varargs parameter.

See scijava/scripting-jython#8.

Author: ctrueden

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

@@ -42,7 +42,6 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 
-import org.scijava.Identifiable;
 import org.scijava.MenuPath;
 import org.scijava.Priority;
 import org.scijava.convert.ConvertService;
@@ -145,8 +144,7 @@ public List<ModuleInfo> getModules() {
 	public ModuleInfo getModuleById(final String id) {
 		// TODO: Cache identifiers in a hash?
 		for (final ModuleInfo info : getModules()) {
-			if (!(info instanceof Identifiable)) continue;
-			final String infoID = ((Identifiable) info).getIdentifier();
+			final String infoID = info.getIdentifier();
 			if (id.equals(infoID)) return info;
 		}
 		return null;
@@ -412,6 +410,26 @@ private Map<String, Object> createMap(final Object[] values) {
 
 		final HashMap<String, Object> inputMap = new HashMap<>();
 
+		if (values.length == 1 && values[0] instanceof Map) {
+			// NB: This hack works around an issue where some script languages,
+			// notably Jython but potentially others too, invoke the wrong run
+			// method when called with a map. The Object... varargs method is
+			// chosen instead of the Map method, with the map being passed as
+			// the sole element of the object array. The code below detects
+			// this situation, propagating the map entries into the new map.
+			final Map<?, ?> valueMap = (Map<?, ?>) values[0];
+			for (final Object key : valueMap.keySet()) {
+				if (!(key instanceof String)) {
+					log.error("Invalid input name: " + key);
+					continue;
+				}
+				final String name = (String) key;
+				final Object value = valueMap.get(key);
+				inputMap.put(name, value);
+			}
+			return inputMap;
+		}
+
 		if (values.length % 2 != 0) {
 			log.error("Ignoring extraneous argument: " + values[values.length - 1]);
 		}

src/test/java/org/scijava/module/ModuleServiceTest.java

@@ -31,9 +31,17 @@
 
 package org.scijava.module;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.ExecutionException;
+
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 import org.scijava.Context;
 
@@ -44,11 +52,89 @@
  */
 public class ModuleServiceTest {
 
-	@Test
-	public void testGetSingleInput() throws ModuleException {
+	private ModuleService moduleService;
+
+	@Before
+	public void setUp() {
 		final Context context = new Context(ModuleService.class);
-		final ModuleService moduleService = context.getService(ModuleService.class);
+		moduleService = context.service(ModuleService.class);
+	}
+
+	@After
+	public void tearDown() {
+		moduleService.context().dispose();
+	}
+
+	/** Tests {@link ModuleService#run(ModuleInfo, boolean, Object...)}. */
+	@Test
+	public void testRunModuleInfoArray() throws InterruptedException,
+		ExecutionException
+	{
+		final ModuleInfo info = new FooModuleInfo();
+		final Module m = moduleService.run(info, false, createInputArray()).get();
+		assertEquals(expectedResult(), m.getOutput("result"));
+	}
+
+	/** Tests {@link ModuleService#run(ModuleInfo, boolean, Map)}. */
+	@Test
+	public void testRunModuleInfoMap() throws InterruptedException,
+		ExecutionException
+	{
+		final ModuleInfo info = new FooModuleInfo();
+		final Module m = moduleService.run(info, false, createInputMap()).get();
+		assertEquals(expectedResult(), m.getOutput("result"));
+	}
+
+	/** Tests {@link ModuleService#run(Module, boolean, Object...)}. */
+	@Test
+	public void testRunModuleArray() throws ModuleException, InterruptedException,
+		ExecutionException
+	{
+		final ModuleInfo info = new FooModuleInfo();
+		final Module module = info.createModule();
+		final Module m = moduleService.run(module, false, createInputArray()).get();
+		assertSame(module, m);
+		assertEquals(expectedResult(), m.getOutput("result"));
+	}
+
+	/** Tests {@link ModuleService#run(Module, boolean, Map)}. */
+	@Test
+	public void testRunModuleMap() throws ModuleException, InterruptedException,
+		ExecutionException
+	{
+		final ModuleInfo info = new FooModuleInfo();
+		final Module module = info.createModule();
+		final Module m = moduleService.run(module, false, createInputMap()).get();
+		assertSame(module, m);
+		assertEquals(expectedResult(), m.getOutput("result"));
+	}
+
+	/**
+	 * Tests that {@link ModuleService#run(ModuleInfo, boolean, Object...)} and
+	 * {@link ModuleService#run(Module, boolean, Object...)} intelligently handle
+	 * a single-element {@link Object} array consisting of a {@code Map<String,
+	 * Object>}.
+	 * <p>
+	 * This situation can happen e.g. due to Jython choosing the wrong overloaded
+	 * {@code run} method. We correct for the issue on our side, for convenience.
+	 * </p>
+	 */
+	@Test
+	public void testRunMapHack() throws ModuleException, InterruptedException,
+		ExecutionException
+	{
+		final ModuleInfo info = new FooModuleInfo();
+		final Object[] inputs = new Object[] { createInputMap() };
+		final Module m = moduleService.run(info, false, inputs).get();
+		assertEquals(expectedResult(), m.getOutput("result"));
+
+		final Module module = info.createModule();
+		final Module m2 = moduleService.run(module, false, inputs).get();
+		assertEquals(expectedResult(), m2.getOutput("result"));
+	}
 
+	@Test
+	public void testGetSingleInput() throws ModuleException {
 		final ModuleInfo info = new FooModuleInfo();
 		final Module module = info.createModule();
 
@@ -74,6 +160,44 @@ public void testGetSingleInput() throws ModuleException {
 		assertSame(info.getInput("double2"), singleDouble);
 	}
 
+	// -- Helper methods --
+
+	private Object[] createInputArray() {
+		return new Object[] { //
+			"string", "hello", //
+			"float", 1.234f, //
+			"integer1", -2, //
+			"integer2", 7, //
+			"double1", Math.E, //
+			"double2", Math.PI //
+		};
+	}
+
+	private Map<String, Object> createInputMap() {
+		final Map<String, Object> inputMap = new HashMap<>();
+		inputMap.put("string", "hello");
+		inputMap.put("float", 1.234f);
+		inputMap.put("integer1", -2);
+		inputMap.put("integer2", 7);
+		inputMap.put("double1", Math.E);
+		inputMap.put("double2", Math.PI);
+		return inputMap;
+	}
+
+	private String expectedResult() {
+		return mapToString(createInputMap());
+	}
+
+	private static String mapToString(final Map<String, Object> map) {
+		final StringBuilder sb = new StringBuilder();
+		for (final Entry<String, Object> entry : map.entrySet()) {
+			sb.append(entry.getKey() + " = " + entry.getValue() + "\n");
+		}
+		return sb.toString();
+	}
+
+	// -- Helper classes --
+
 	/** A sample module for testing the module service. */
 	public static class FooModule extends AbstractModule {
 
@@ -90,7 +214,7 @@ public ModuleInfo getInfo() {
 
 		@Override
 		public void run() {
-			// TODO Auto-generated method stub
+			setOutput("result", mapToString(getInputs()));
 		}
 
 	}
@@ -121,6 +245,7 @@ protected void parseParameters() {
 			addInput("integer2", Integer.class, true);
 			addInput("double1", Double.class, false);
 			addInput("double2", Double.class, true);
+			addOutput("result", String.class);
 		}
 
 		private <T> void addInput(final String name, final Class<T> type,
@@ -146,6 +271,22 @@ public boolean isAutoFill() {
 			});
 		}
 
+		private <T> void addOutput(final String name, final Class<T> type) {
+			registerOutput(new AbstractModuleItem<T>(this) {
+
+				@Override
+				public String getName() {
+					return name;
+				}
+
+				@Override
+				public Class<T> getType() {
+					return type;
+				}
+
+			});
+		}
+
 	}
 
 }