Fix Logger Parameter with Ops

src/main/java/org/scijava/Context.java

@@ -45,7 +45,9 @@
 import org.scijava.event.EventHandler;
 import org.scijava.event.EventService;
 import org.scijava.log.LogService;
+import org.scijava.log.Logger;
 import org.scijava.plugin.Parameter;
+import org.scijava.plugin.Plugin;
 import org.scijava.plugin.PluginIndex;
 import org.scijava.service.Service;
 import org.scijava.service.ServiceHelper;
@@ -495,6 +497,22 @@ else if (Context.class.isAssignableFrom(type) && type.isInstance(this)) {
 				// populate Context parameter
 				ClassUtils.setValue(f, o, this);
 			}
+			else if (Logger.class.isAssignableFrom(type)) {
+				final Logger existingLogger = (Logger) ClassUtils.getValue(f, o);
+				if (existingLogger == null) {
+					final LogService logService = getService(LogService.class);
+					if(logService != null) {
+						Parameter annotation = f.getAnnotation(Parameter.class);
+						String label = annotation.label();
+						String name = label == null || label.isEmpty() ? o.getClass().getSimpleName() : label;
+						final Logger logger = logService.subLogger(name);
+						ClassUtils.setValue(f, o, logger);
+					} else if(f.getAnnotation(Parameter.class).required()) {
+						throw new IllegalArgumentException(
+								createMissingServiceMessage(LogService.class));
+					}
+				}
+			}
 			else if (!type.isPrimitive()) {
 				// the parameter is some other object; if it is non-null, we recurse
 				final Object value = ClassUtils.getValue(f, o);

src/main/java/org/scijava/log/Logger.java

@@ -31,6 +31,8 @@
 
 package org.scijava.log;
 
+import org.scijava.Optional;
+
 import static org.scijava.log.LogLevel.DEBUG;
 import static org.scijava.log.LogLevel.ERROR;
 import static org.scijava.log.LogLevel.INFO;
@@ -49,7 +51,7 @@
  * @see LogService
  */
 @IgnoreAsCallingClass
-public interface Logger {
+public interface Logger extends Optional {
 
 	default void debug(final Object msg) {
 		log(DEBUG, msg);

src/test/java/org/scijava/log/LoggerInjectionTest.java

@@ -0,0 +1,89 @@
+package org.scijava.log;
+
+import org.junit.Test;
+import org.scijava.Context;
+import org.scijava.plugin.Parameter;
+
+import java.util.Collections;
+
+import static junit.framework.TestCase.assertNotNull;
+import static junit.framework.TestCase.assertTrue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+public class LoggerInjectionTest {
+
+	private final Context context = new Context(LogService.class);
+
+	@Test
+	public void testInjection() {
+		// setup
+		LogService logService = context.service(LogService.class);
+		TestLogListener listener = new TestLogListener();
+		logService.addLogListener(listener);
+		// process
+		ObjectWithLogger object = new ObjectWithLogger();
+		context.inject(object);
+		object.logSomething();
+		// test
+		assertTrue(listener.hasLogged(m -> "Something".equals(m.text())));
+	}
+
+	@Test
+	public void testDefaultLoggerName() {
+		ObjectWithLogger object = new ObjectWithLogger();
+		context.inject(object);
+		assertEquals(ObjectWithLogger.class.getSimpleName(), object.getLogger().getName());
+	}
+
+	@Test
+	public void testCustomLoggerName() {
+		ObjectWithLabeledLogger object = new ObjectWithLabeledLogger();
+		context.inject(object);
+		assertEquals("xyz", object.getLogger().getName());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testMissingLogService() {
+		Context emptyContext = new Context(Collections.emptyList());
+		ObjectWithLogger object = new ObjectWithLogger();
+		emptyContext.inject(object);
+	}
+
+	@Test
+	public void testMissingLogServiceOptionalLogger() {
+		Context emptyContext = new Context(Collections.emptyList());
+		ObjectWithOptionalLogger object = new ObjectWithOptionalLogger();
+		emptyContext.inject(object);
+		assertNull(object.getLogger());
+	}
+
+	public static class ObjectWithLogger {
+
+		@Parameter Logger log;
+
+		public Logger getLogger() {
+			return log;
+		}
+
+		public void logSomething() { log.warn("Something"); }
+	}
+
+	public static class ObjectWithLabeledLogger {
+
+		@Parameter(label = "xyz") Logger log;
+
+		public Logger getLogger() {
+			return log;
+		}
+	}
+
+	public static class ObjectWithOptionalLogger {
+
+		@Parameter(required = false) Logger log;
+
+		public Logger getLogger() {
+			return log;
+		}
+	}
+}

src/test/java/org/scijava/module/process/LoggerPreprocessorTest.java → src/test/java/org/scijava/module/process/CommandServiceLoggerIntegrationTest.java

@@ -39,6 +39,7 @@
 import org.junit.Test;
 import org.scijava.Context;
 import org.scijava.command.Command;
+import org.scijava.command.CommandInfo;
 import org.scijava.command.CommandService;
 import org.scijava.log.DefaultLogger;
 import org.scijava.log.LogLevel;
@@ -49,40 +50,41 @@
 import org.scijava.plugin.Parameter;
 
 /**
- * Tests {@link LoggerPreprocessor}.
+ * Tests logger injection with {@link CommandService}
  *
  * @author Matthias Arzt
  */
-public class LoggerPreprocessorTest {
+public class CommandServiceLoggerIntegrationTest {
 
+	private final Context context = new Context(CommandService.class);
+	private final CommandService commandService = context.service(CommandService.class);
+	private final LogService service = context.service(LogService.class);
+	private static final String MESSAGE_TEXT = "foobar";
+
+	/** Test logging, when no logger is explicitly given to {@link CommandService#run} */
 	@Test
 	public void testInjection() throws InterruptedException, ExecutionException {
-		final Context context = new Context(CommandService.class);
-		final CommandService commandService = context.service(CommandService.class);
+		// setup
 		final TestLogListener listener = new TestLogListener();
-		context.service(LogService.class).addLogListener(listener);
-
+		service.addLogListener(listener);
+		// process
 		commandService.run(CommandWithLogger.class, true).get();
-		assertTrue(listener.hasLogged(m -> m.source().path().contains(CommandWithLogger.class.getSimpleName())));
+		// test
+		assertTrue(listener.hasLogged(m -> MESSAGE_TEXT.equals(m.text())));
 	}
 
 	/** Tests redirection of a command's log output. */
 	@Test
-	public void testCustomLogger() throws ExecutionException,
-		InterruptedException
-	{
+	public void testCustomLogger() throws ExecutionException, InterruptedException {
 		// setup
-		final Context context = new Context(CommandService.class);
-		final CommandService commandService = context.service(CommandService.class);
 		final TestLogListener listener = new TestLogListener();
-		final LogSource source = LogSource.newRoot();
-		final DefaultLogger customLogger = new DefaultLogger(listener, source,
-			LogLevel.TRACE);
+		final LogSource customSource = LogSource.newRoot();
+		final DefaultLogger customLogger = new DefaultLogger(listener, customSource, LogLevel.TRACE);
 		// process
 		commandService.run(CommandWithLogger.class, true, "log", customLogger)
 			.get();
 		// test
-		assertTrue(listener.hasLogged(m -> m.source().equals(source)));
+		assertTrue(listener.hasLogged(m -> m.source().equals(customSource)));
 	}
 
 	public static class CommandWithLogger implements Command {
@@ -92,25 +94,7 @@ public static class CommandWithLogger implements Command {
 
 		@Override
 		public void run() {
-			log.info("log from the command.");
-		}
-	}
-
-	@Test
-	public void testLoggerNameByAnnotation() throws ExecutionException, InterruptedException {
-		final Context context = new Context(CommandService.class);
-		final CommandService commandService = context.service(CommandService.class);
-		commandService.run(CommandWithNamedLogger.class, true).get();
-	}
-
-	public static class CommandWithNamedLogger implements Command {
-
-		@Parameter(label = "MyLoggerName")
-		public Logger log;
-
-		@Override
-		public void run() {
-			assertEquals("MyLoggerName", log.getName());
+			log.info(MESSAGE_TEXT);
 		}
 	}
 }