Merge pull request #38 from maarzt/fix-logformatter-preferences

LogFormatter: fix the storage of preferences with arbitrary key value

Author: ctrueden

src/main/java/org/scijava/ui/swing/console/ConsolePanel.java

@@ -47,9 +47,11 @@
 
 import net.miginfocom.swing.MigLayout;
 
+import org.scijava.Context;
 import org.scijava.console.OutputEvent;
 import org.scijava.console.OutputListener;
 import org.scijava.log.IgnoreAsCallingClass;
+import org.scijava.plugin.Parameter;
 import org.scijava.thread.ThreadService;
 import org.scijava.ui.swing.StaticSwingUtils;
 
@@ -73,10 +75,11 @@ public class ConsolePanel extends JPanel implements OutputListener
 	private Style stdoutGlobal;
 	private Style stderrGlobal;
 
-	private final ThreadService threadService;
+	@Parameter
+	private ThreadService threadService;
 
-	public ConsolePanel(ThreadService threadService) {
-		this.threadService = threadService;
+	public ConsolePanel(final Context context) {
+		context.inject(this);
 		initGui();
 	}
 

src/main/java/org/scijava/ui/swing/console/ItemTextPane.java

@@ -33,7 +33,6 @@
 import java.awt.Font;
 import java.util.Collections;
 import java.util.Iterator;
-import java.util.Objects;
 
 import javax.swing.JComponent;
 import javax.swing.JPopupMenu;
@@ -44,6 +43,8 @@
 import javax.swing.text.DefaultStyledDocument;
 import javax.swing.text.StyledDocument;
 
+import org.scijava.Context;
+import org.scijava.plugin.Parameter;
 import org.scijava.thread.ThreadService;
 import org.scijava.ui.swing.StaticSwingUtils;
 
@@ -66,8 +67,6 @@
  */
 class ItemTextPane {
 
-	private final ThreadService threadService;
-
 	private DocumentCalculator initialCalculator = null;
 
 	private JTextPane textPane = new JTextPane();
@@ -79,10 +78,13 @@ class ItemTextPane {
 	private DocumentCalculator calculator =
 		new DocumentCalculator(Collections.<Item> emptyList().iterator());
 
+	@Parameter
+	private ThreadService threadService;
+
 	// -- constructor --
 
-	ItemTextPane(ThreadService threadService) {
-		this.threadService = Objects.requireNonNull(threadService);
+	ItemTextPane(final Context context) {
+		context.inject(this);
 		textPane.setEditable(false);
 		textPane.setFont(new Font("monospaced", Font.PLAIN, 12));
 	}

src/main/java/org/scijava/ui/swing/console/LogFormatter.java

@@ -36,8 +36,10 @@
 import java.util.EnumSet;
 import java.util.Map;
 
+import org.scijava.Context;
 import org.scijava.log.LogLevel;
 import org.scijava.log.LogMessage;
+import org.scijava.plugin.Parameter;
 import org.scijava.prefs.PrefService;
 
 /**
@@ -47,12 +49,13 @@
  */
 public class LogFormatter {
 
-	private String prefKey = null;
+	private final String prefKey;
 
-	private PrefService prefService = null;
+	@Parameter(required = false)
+	private PrefService prefService;
 
-	public void setPrefService(PrefService prefService, String prefKey) {
-		this.prefService = (prefKey != null && !prefKey.isEmpty()) ? prefService : null;
+	public LogFormatter(final Context context, final String prefKey) {
+		context.inject(this);
 		this.prefKey = prefKey;
 		applySettings();
 	}
@@ -114,8 +117,9 @@ private void printWithBrackets(PrintWriter printer, String prefix) {
 	// -- Helper methods --
 
 	public void applySettings() {
-		if(prefService == null) return;
-		Map<String, String> settings = prefService.getMap(prefKey);
+		if (skipPersist()) return;
+		final Map<String, String> settings = //
+			prefService.getMap(LogFormatter.class, prefKey);
 		for(Field field : Field.values()) {
 			String defaultValue = Boolean.toString(isVisible(field));
 			String value = settings.getOrDefault(field.toString(), defaultValue);
@@ -125,9 +129,13 @@ public void applySettings() {
 	}
 
 	public void changeSetting(Field field, boolean visible) {
-		if(prefService == null) return;
-		Map<String, String> settings = prefService.getMap(prefKey);
+		if (skipPersist()) return;
+		Map<String, String> settings = prefService.getMap(LogFormatter.class, prefKey);
 		settings.put(field.toString(), Boolean.toString(visible));
-		prefService.putMap(prefKey, settings);
+		prefService.put(LogFormatter.class, prefKey, settings);
+	}
+
+	private boolean skipPersist() {
+		return prefService == null || prefKey == null || prefKey.isEmpty();
 	}
 }

src/main/java/org/scijava/ui/swing/console/LoggingPanel.java

@@ -71,6 +71,7 @@
 import org.scijava.log.LogService;
 import org.scijava.log.LogSource;
 import org.scijava.log.Logger;
+import org.scijava.plugin.Parameter;
 import org.scijava.prefs.PrefService;
 import org.scijava.thread.ThreadService;
 
@@ -87,7 +88,6 @@
 @IgnoreAsCallingClass
 public class LoggingPanel extends JPanel implements LogListener
 {
-
 	private static final AttributeSet STYLE_ERROR = normal(new Color(200, 0, 0));
 	private static final AttributeSet STYLE_WARN = normal(new Color(200, 140, 0));
 	private static final AttributeSet STYLE_INFO = normal(Color.BLACK);
@@ -107,19 +107,26 @@ public class LoggingPanel extends JPanel implements LogListener
 
 	private final Set<LogSource> sources = Collections.newSetFromMap(
 		new ConcurrentHashMap<>());
-	private final LogFormatter logFormatter = new LogFormatter();
+	private final LogFormatter logFormatter;
 
 	private LogRecorder recorder;
 
+	@Parameter
+	private ThreadService threadService;
+
+	@Parameter
+	private PrefService prefService;
+
 	// -- constructor --
 
-	public LoggingPanel(Context context) {
-		this(context.getService(ThreadService.class), null, null);
+	public LoggingPanel(final Context context) {
+		this(context, null);
 	}
 
-	public LoggingPanel(ThreadService threadService, PrefService prefService, String prefKey) {
-		textArea = new ItemTextPane(threadService);
-		logFormatter.setPrefService(prefService, prefKey);
+	public LoggingPanel(final Context context, final String prefKey) {
+		context.inject(this);
+		textArea = new ItemTextPane(context);
+		logFormatter = new LogFormatter(context, prefKey);
 		initGui();
 		setRecorder(new LogRecorder());
 	}
@@ -130,7 +137,7 @@ public void setRecorder(LogRecorder recorder) {
 		if (recorder != null) recorder.removeObserver(textArea::update);
 		this.recorder = recorder;
 		updateFilter();
-		recorder.addObservers(textArea::update);
+		if (recorder != null) recorder.addObservers(textArea::update);
 	}
 
 	public void toggleSourcesPanel() {

src/main/java/org/scijava/ui/swing/console/SwingConsolePane.java

@@ -1,4 +1,5 @@
 /*
+ * 
  * #%L
  * SciJava UI components for Java Swing.
  * %%
@@ -59,7 +60,14 @@
  */
 public class SwingConsolePane extends AbstractConsolePane<JPanel> {
 
-	public static final String LOG_FORMATTING_SETTINGS_KEY = "/log-formatting";
+	/**
+	 * Key to use when when persisting {@link LogFormatter} preferences with the
+	 * {@link PrefService}.
+	 */
+	private static final String LOG_FORMATTING_SETTINGS_KEY = "log-formatting";
+
+	@Parameter
+	private Context context;
 
 	@Parameter
 	private ThreadService threadService;
@@ -138,8 +146,8 @@ private ConsolePanel consolePanel() {
 
 	private synchronized void initLoggingPanel() {
 		if (consolePanel != null) return;
-		consolePanel = new ConsolePanel(threadService);
-		loggingPanel = new LoggingPanel(threadService, prefService, LOG_FORMATTING_SETTINGS_KEY);
+		consolePanel = new ConsolePanel(context);
+		loggingPanel = new LoggingPanel(context, LOG_FORMATTING_SETTINGS_KEY);
 		logService.addLogListener(loggingPanel);
 		component = new JPanel(new MigLayout("", "[grow]", "[grow]"));
 		JTabbedPane tabs = new JTabbedPane();

src/test/java/org/scijava/ui/swing/console/LogFormatterTest.java

@@ -1,12 +1,8 @@
 package org.scijava.ui.swing.console;
 
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
-import java.util.Collections;
-import java.util.Map;
-
 import org.junit.Test;
 import org.scijava.Context;
 import org.scijava.prefs.PrefService;
@@ -18,34 +14,27 @@
  */
 public class LogFormatterTest {
 
-	@Test
-	public void testPrefService() {
-		PrefService prefService = new Context(PrefService.class).service(PrefService.class);
-		String expected = "Hello World";
-		prefService.put("foo", expected);
-		String actual = prefService.get("foo");
-		assertEquals(expected, actual);
-	}
-
-	@Test
-	public void testPrefServiceMap() {
-		PrefService prefService = new Context(PrefService.class).service(PrefService.class);
-		Map<String, String> expected = Collections.singletonMap("Hello", "World");
-		prefService.putMap("/foo", expected);
-		Map<String, String> actual = prefService.getMap("/foo");
-		assertEquals(expected, actual);
-	}
-
 	@Test
 	public void testSettings() {
-		PrefService prefService = new Context(PrefService.class).service(PrefService.class);
-		LogFormatter formatter1 = new LogFormatter();
-		formatter1.setPrefService(prefService, "/abc");
+		Context context = new Context(PrefService.class);
+		final String usedKey = "LogFormatterTest-key";
+		final String nonMatchingKey = "LogFormatterTest-nonmatching";
+		context.service(PrefService.class).remove(LogFormatter.class, usedKey);
+		context.service(PrefService.class).remove(LogFormatter.class, nonMatchingKey);
+
+		// Assign some settings to a particular key.
+		LogFormatter formatter1 = new LogFormatter(context, usedKey);
 		formatter1.setVisible(LogFormatter.Field.ATTACHMENT, true);
 		formatter1.setVisible(LogFormatter.Field.LEVEL, false);
-		LogFormatter formatter2 = new LogFormatter();
-		formatter2.setPrefService(prefService, "/abc");
+
+		// Check that matching key does share these settings.
+		LogFormatter formatter2 = new LogFormatter(context, usedKey);
 		assertTrue(formatter2.isVisible(LogFormatter.Field.ATTACHMENT));
 		assertFalse(formatter2.isVisible(LogFormatter.Field.LEVEL));
+
+		// Check that non-matching key does not share these settings.
+		LogFormatter formatter3 = new LogFormatter(context, nonMatchingKey);
+		assertFalse(formatter3.isVisible(LogFormatter.Field.ATTACHMENT));
+		assertTrue(formatter3.isVisible(LogFormatter.Field.LEVEL));
 	}
 }