diff --git a/src/main/java/org/scijava/Context.java b/src/main/java/org/scijava/Context.java index c3b928c2d..b47562abe 100644 --- a/src/main/java/org/scijava/Context.java +++ b/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); diff --git a/src/main/java/org/scijava/log/Logger.java b/src/main/java/org/scijava/log/Logger.java index bc0f3203e..d9a56f124 100644 --- a/src/main/java/org/scijava/log/Logger.java +++ b/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); diff --git a/src/main/java/org/scijava/module/process/LoggerPreprocessor.java b/src/main/java/org/scijava/module/process/LoggerPreprocessor.java deleted file mode 100644 index 2e62faaf1..000000000 --- a/src/main/java/org/scijava/module/process/LoggerPreprocessor.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * #%L - * SciJava Common shared library for SciJava software. - * %% - * Copyright (C) 2009 - 2017 Board of Regents of the University of - * Wisconsin-Madison, Broad Institute of MIT and Harvard, and Max Planck - * Institute of Molecular Cell Biology and Genetics. - * %% - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - * #L% - */ - -package org.scijava.module.process; - -import org.scijava.log.LogService; -import org.scijava.log.Logger; -import org.scijava.module.Module; -import org.scijava.module.ModuleItem; -import org.scijava.module.ModuleService; -import org.scijava.plugin.Parameter; -import org.scijava.plugin.Plugin; - -/** - * This {@link PreprocessorPlugin} affects {@link Module}s with a single - * {@link Parameter} of type {@link Logger}. It will assign a Logger to that - * Parameter, that is named like the modules class. - * - * @author Matthias Arzt - */ -@Plugin(type = PreprocessorPlugin.class) -public class LoggerPreprocessor extends AbstractPreprocessorPlugin { - - @Parameter(required = false) - private LogService logService; - - @Parameter(required = false) - private ModuleService moduleService; - - // -- ModuleProcessor methods -- - - @Override - public void process(final Module module) { - if (logService == null || moduleService == null) return; - - final ModuleItem loggerInput = moduleService.getSingleInput(module, - Logger.class); - if (loggerInput == null || !loggerInput.isAutoFill()) return; - - String loggerName = loggerInput.getLabel(); - if(loggerName == null || loggerName.isEmpty()) - loggerName = module.getDelegateObject().getClass().getSimpleName(); - Logger logger = logService.subLogger(loggerName); - - final String name = loggerInput.getName(); - module.setInput(name, logger); - module.resolveInput(name); - } - -} diff --git a/src/test/java/org/scijava/log/LoggerInjectionTest.java b/src/test/java/org/scijava/log/LoggerInjectionTest.java new file mode 100644 index 000000000..aefc80bdf --- /dev/null +++ b/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; + } + } +} diff --git a/src/test/java/org/scijava/module/process/LoggerPreprocessorTest.java b/src/test/java/org/scijava/module/process/CommandServiceLoggerIntegrationTest.java similarity index 68% rename from src/test/java/org/scijava/module/process/LoggerPreprocessorTest.java rename to src/test/java/org/scijava/module/process/CommandServiceLoggerIntegrationTest.java index b3cbc60ca..a5f66fb29 100644 --- a/src/test/java/org/scijava/module/process/LoggerPreprocessorTest.java +++ b/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); } } }