fix(ui): keep the saved sidebar tab and stdout intact while closing

runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java

@@ -168,6 +168,7 @@ public class ClientUI
 	private boolean consoleVisible;
 	private boolean sidebarTabRestored;
 	private boolean updatingNavigation;
+	private boolean closing;
 
 	@RequiredArgsConstructor
 	private static class HistoryEntry
@@ -278,7 +279,10 @@ void removeNavigation(NavigationButton navBtn)
 						: selectedTabHistory.removeLast();
 
 					openPanel(entry.navBtn, entry.sidebarOpen);
-					saveSidebarTab();
+					if (!closing)
+					{
+						saveSidebarTab();
+					}
 				}
 			}
 			finally
@@ -290,6 +294,12 @@ void removeNavigation(NavigationButton navBtn)
 		sidebarEntries.remove(navBtn);
 	}
 
+	@Subscribe(priority = 100)
+	private void onClientShutdown(ClientShutdown event)
+	{
+		closing = true;
+	}
+
 	@Subscribe
 	private void onGameStateChanged(final GameStateChanged event)
 	{

runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java

@@ -12,6 +12,8 @@
 import ch.qos.logback.core.AppenderBase;
 import ch.qos.logback.core.ConsoleAppender;
 import ch.qos.logback.core.joran.spi.ConsoleTarget;
+import java.io.FilterOutputStream;
+import java.io.IOException;
 import java.io.OutputStream;
 import java.util.Iterator;
 import java.util.function.Consumer;
@@ -28,6 +30,7 @@ final class ConsoleLogAppender extends AppenderBase<ILoggingEvent>
 
 	static void bindStdoutAppenders(LoggerContext context, OutputStream stdout)
 	{
+		OutputStream shielded = new CloseShieldOutputStream(stdout);
 		for (Logger logger : context.getLoggerList())
 		{
 			Iterator<Appender<ILoggingEvent>> appenders = logger.iteratorForAppenders();
@@ -37,7 +40,7 @@ static void bindStdoutAppenders(LoggerContext context, OutputStream stdout)
 				if (appender instanceof ConsoleAppender
 					&& ConsoleTarget.SystemOut.getName().equals(((ConsoleAppender<ILoggingEvent>) appender).getTarget()))
 				{
-					((ConsoleAppender<ILoggingEvent>) appender).setOutputStream(stdout);
+					((ConsoleAppender<ILoggingEvent>) appender).setOutputStream(shielded);
 				}
 			}
 		}
@@ -63,4 +66,24 @@ protected void append(ILoggingEvent eventObject)
 		String formatted = layout.doLayout(eventObject);
 		logConsumer.accept(formatted);
 	}
+
+	private static final class CloseShieldOutputStream extends FilterOutputStream
+	{
+		private CloseShieldOutputStream(OutputStream out)
+		{
+			super(out);
+		}
+
+		@Override
+		public void write(byte[] b, int off, int len) throws IOException
+		{
+			out.write(b, off, len);
+		}
+
+		@Override
+		public void close() throws IOException
+		{
+			flush();
+		}
+	}
 }

runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java

@@ -0,0 +1,250 @@
+package net.runelite.client.ui;
+
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.ConsoleAppender;
+import ch.qos.logback.core.joran.spi.ConsoleTarget;
+import com.google.common.collect.Iterables;
+import java.awt.image.BufferedImage;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
+import java.util.Collection;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.inject.Provider;
+import javax.swing.JScrollPane;
+import javax.swing.JTabbedPane;
+import javax.swing.JTextArea;
+import javax.swing.SwingUtilities;
+import javax.swing.text.Document;
+import net.runelite.api.Client;
+import net.runelite.client.callback.ClientThread;
+import net.runelite.client.config.ConfigManager;
+import net.runelite.client.config.RuneLiteConfig;
+import net.runelite.client.eventbus.EventBus;
+import net.runelite.client.events.ClientShutdown;
+import net.runelite.client.input.MouseManager;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.LoggerFactory;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+public class ClientUITest
+{
+	private static final String CONFIG_GROUP = "runelite";
+	private static final String SIDEBAR_TAB = "clientSidebarTab";
+
+	private ConfigManager configManager;
+	private EventBus eventBus;
+	private ClientUI clientUI;
+	private PrintStream originalOut;
+	private ConsoleAppender<ILoggingEvent> stdoutAppender;
+
+	@Before
+	public void setUp() throws Exception
+	{
+		originalOut = System.out;
+		configManager = mock(ConfigManager.class);
+		eventBus = new EventBus();
+		Constructor<ClientUI> constructor = ClientUI.class.getDeclaredConstructor(RuneLiteConfig.class, MouseManager.class,
+			Client.class, ConfigManager.class, Provider.class, EventBus.class, boolean.class, String.class);
+		constructor.setAccessible(true);
+		clientUI = constructor.newInstance(mock(RuneLiteConfig.class), null, null, configManager,
+			(Provider<ClientThread>) () -> null, eventBus, false, "Microbot");
+		eventBus.register(clientUI);
+	}
+
+	@After
+	public void tearDown() throws Exception
+	{
+		System.setOut(originalOut);
+		LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory();
+		Logger root = context.getLogger(Logger.ROOT_LOGGER_NAME);
+		ConsoleLogAppender consoleLogAppender = (ConsoleLogAppender) get("consoleLogAppender");
+		if (consoleLogAppender != null)
+		{
+			root.detachAppender(consoleLogAppender);
+			consoleLogAppender.stop();
+		}
+		if (stdoutAppender != null)
+		{
+			root.detachAppender(stdoutAppender);
+			stdoutAppender.stop();
+			ConsoleLogAppender.bindStdoutAppenders(context, ConsoleTarget.SystemOut.getStream());
+		}
+	}
+
+	@Test
+	public void removingTheOpenTabStoresTheTabShownInstead() throws Exception
+	{
+		NavigationButton first = button("First", 1);
+		NavigationButton second = button("Second", 2);
+		onEdt(() ->
+		{
+			installSidebar();
+			clientUI.addNavigation(first);
+			clientUI.addNavigation(second);
+			clientUI.openPanel(first, true);
+			clientUI.openPanel(second, true);
+			clientUI.removeNavigation(second);
+		});
+
+		verify(configManager).setConfiguration(CONFIG_GROUP, SIDEBAR_TAB, "First");
+	}
+
+	@Test
+	public void removingTheOpenTabWhileClosingKeepsTheSavedTab() throws Exception
+	{
+		NavigationButton first = button("First", 1);
+		NavigationButton second = button("Second", 2);
+		onEdt(() ->
+		{
+			installSidebar();
+			clientUI.addNavigation(first);
+			clientUI.addNavigation(second);
+			clientUI.openPanel(first, true);
+			clientUI.openPanel(second, true);
+		});
+
+		eventBus.post(new ClientShutdown());
+		onEdt(() -> clientUI.removeNavigation(second));
+
+		verify(configManager, never()).setConfiguration(eq(CONFIG_GROUP), eq(SIDEBAR_TAB), any(String.class));
+		verify(configManager, never()).unsetConfiguration(CONFIG_GROUP, SIDEBAR_TAB);
+	}
+
+	@Test
+	public void consoleInitializationKeepsStdoutLogLinesOutOfThePanel() throws Exception
+	{
+		ByteArrayOutputStream terminal = new ByteArrayOutputStream();
+		System.setOut(new PrintStream(terminal, true, StandardCharsets.UTF_8));
+
+		LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory();
+		PatternLayoutEncoder encoder = new PatternLayoutEncoder();
+		encoder.setContext(context);
+		encoder.setPattern("%msg%n");
+		encoder.start();
+		stdoutAppender = new ConsoleAppender<>();
+		stdoutAppender.setContext(context);
+		stdoutAppender.setEncoder(encoder);
+		stdoutAppender.start();
+		Logger root = context.getLogger(Logger.ROOT_LOGGER_NAME);
+		root.addAppender(stdoutAppender);
+
+		AtomicReference<LogConsolePanel> panel = new AtomicReference<>();
+		onEdt(() -> panel.set(new LogConsolePanel()));
+		set("consolePanel", panel.get());
+		Method initialize = ClientUI.class.getDeclaredMethod("initializeConsoleLogging");
+		initialize.setAccessible(true);
+		initialize.invoke(clientUI);
+
+		String line = "console init probe " + System.nanoTime();
+		root.warn(line);
+		onEdt(() -> { });
+		onEdt(() -> { });
+
+		JTextArea textArea = (JTextArea) ((JScrollPane) panel.get().getComponent(0)).getViewport().getView();
+		AtomicReference<String> text = new AtomicReference<>();
+		onEdt(() ->
+		{
+			Document document = textArea.getDocument();
+			text.set(document.getText(0, document.getLength()));
+		});
+
+		assertEquals(1, occurrences(text.get(), line));
+		assertTrue(terminal.toString(StandardCharsets.UTF_8).contains(line));
+	}
+
+	private void installSidebar() throws Exception
+	{
+		JTabbedPane sidebar = new JTabbedPane(JTabbedPane.RIGHT);
+		sidebar.setSelectedIndex(-1);
+		Collection<NavigationButton> entries = (Collection<NavigationButton>) get("sidebarEntries");
+		sidebar.addChangeListener(ev ->
+		{
+			int index = sidebar.getSelectedIndex();
+			try
+			{
+				set("selectedTab", index < 0 ? null : Iterables.get(entries, index));
+			}
+			catch (ReflectiveOperationException ex)
+			{
+				throw new IllegalStateException(ex);
+			}
+		});
+		set("sidebar", sidebar);
+	}
+
+	private static NavigationButton button(String tooltip, int priority)
+	{
+		return NavigationButton.builder()
+			.tooltip(tooltip)
+			.priority(priority)
+			.icon(new BufferedImage(16, 16, BufferedImage.TYPE_INT_ARGB))
+			.panel(new PluginPanel()
+			{
+			})
+			.build();
+	}
+
+	private Object get(String name) throws ReflectiveOperationException
+	{
+		Field field = ClientUI.class.getDeclaredField(name);
+		field.setAccessible(true);
+		return field.get(clientUI);
+	}
+
+	private void set(String name, Object value) throws ReflectiveOperationException
+	{
+		Field field = ClientUI.class.getDeclaredField(name);
+		field.setAccessible(true);
+		field.set(clientUI, value);
+	}
+
+	private static int occurrences(String text, String needle)
+	{
+		int count = 0;
+		for (int i = text.indexOf(needle); i >= 0; i = text.indexOf(needle, i + 1))
+		{
+			count++;
+		}
+		return count;
+	}
+
+	private static void onEdt(ThrowingRunnable runnable) throws Exception
+	{
+		AtomicReference<Exception> failure = new AtomicReference<>();
+		SwingUtilities.invokeAndWait(() ->
+		{
+			try
+			{
+				runnable.run();
+			}
+			catch (Exception ex)
+			{
+				failure.set(ex);
+			}
+		});
+		if (failure.get() != null)
+		{
+			throw failure.get();
+		}
+	}
+
+	private interface ThrowingRunnable
+	{
+		void run() throws Exception;
+	}
+}

runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java

@@ -14,6 +14,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 
 public class ConsoleLogAppenderTest
 {
@@ -72,6 +73,30 @@ public void logLineReachesTheConsoleOnceAndStdoutOnce()
 		assertEquals(1, occurrences(out, "plain print"));
 	}
 
+	@Test
+	public void stoppingTheLoggerContextLeavesStdoutOpen()
+	{
+		CloseTrackingStream terminal = new CloseTrackingStream();
+
+		Logger root = context.getLogger(Logger.ROOT_LOGGER_NAME);
+		PatternLayoutEncoder encoder = new PatternLayoutEncoder();
+		encoder.setContext(context);
+		encoder.setPattern("%msg%n");
+		encoder.start();
+		ConsoleAppender<ILoggingEvent> stdout = new ConsoleAppender<>();
+		stdout.setContext(context);
+		stdout.setEncoder(encoder);
+		stdout.start();
+		root.addAppender(stdout);
+
+		ConsoleLogAppender.bindStdoutAppenders(context, terminal);
+		root.info("before stop");
+		context.stop();
+
+		assertFalse(terminal.closed);
+		assertEquals(1, occurrences(terminal.toString(StandardCharsets.UTF_8), "before stop"));
+	}
+
 	private static int occurrences(String text, String needle)
 	{
 		int count = 0;
@@ -82,6 +107,17 @@ private static int occurrences(String text, String needle)
 		return count;
 	}
 
+	private static final class CloseTrackingStream extends ByteArrayOutputStream
+	{
+		private boolean closed;
+
+		@Override
+		public void close()
+		{
+			closed = true;
+		}
+	}
+
 	private static final class Tee extends OutputStream
 	{
 		private final OutputStream first;