diff --git a/runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java b/runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java index 54d64977db0..1befeff5824 100644 --- a/runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java +++ b/runelite-client/src/main/java/net/runelite/client/ui/ClientUI.java @@ -30,7 +30,6 @@ import com.google.common.base.Strings; import com.google.common.collect.Iterables; import com.google.inject.Inject; -import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; import lombok.Getter; import lombok.RequiredArgsConstructor; @@ -278,7 +277,7 @@ void removeNavigation(NavigationButton navBtn) ? new HistoryEntry(true, null) : selectedTabHistory.removeLast(); - openPanel(entry.navBtn, entry.sidebarOpen); + openPanel(entry.navBtn, closing ? sidebar.isVisible() : entry.sidebarOpen); if (!closing) { saveSidebarTab(); @@ -1311,13 +1310,7 @@ private void initializeConsoleLogging() System.setOut(consolePrintStream); LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory(); - ConsoleLogAppender.bindStdoutAppenders(loggerContext, originalOut); - consoleLogAppender = new ConsoleLogAppender(consolePanel::append); - consoleLogAppender.setContext(loggerContext); - consoleLogAppender.start(); - - Logger rootLogger = loggerContext.getLogger(Logger.ROOT_LOGGER_NAME); - rootLogger.addAppender(consoleLogAppender); + consoleLogAppender = ConsoleLogAppender.install(loggerContext, originalOut, consolePanel::append); } private BufferedImage createConsoleIcon(boolean active) diff --git a/runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java b/runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java index 8cc70aadd7b..47e7c328a59 100644 --- a/runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java +++ b/runelite-client/src/main/java/net/runelite/client/ui/ConsoleLogAppender.java @@ -4,10 +4,12 @@ */ package net.runelite.client.ui; +import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.PatternLayout; import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.classic.spi.LoggerContextListener; import ch.qos.logback.core.Appender; import ch.qos.logback.core.AppenderBase; import ch.qos.logback.core.ConsoleAppender; @@ -18,19 +20,47 @@ import java.util.Iterator; import java.util.function.Consumer; -final class ConsoleLogAppender extends AppenderBase +final class ConsoleLogAppender extends AppenderBase implements LoggerContextListener { private final Consumer logConsumer; + private final OutputStream stdout; private PatternLayout layout; + private volatile boolean rebindPending; ConsoleLogAppender(Consumer logConsumer) + { + this(logConsumer, null); + } + + private ConsoleLogAppender(Consumer logConsumer, OutputStream stdout) { this.logConsumer = logConsumer; + this.stdout = stdout; + } + + static ConsoleLogAppender install(LoggerContext context, OutputStream stdout, Consumer logConsumer) + { + ConsoleLogAppender appender = new ConsoleLogAppender(logConsumer, stdout); + appender.setContext(context); + bindStdoutAppenders(context, stdout); + appender.start(); + context.addListener(appender); + context.getLogger(Logger.ROOT_LOGGER_NAME).addAppender(appender); + return appender; + } + + void uninstall() + { + LoggerContext context = (LoggerContext) getContext(); + context.removeListener(this); + context.getLogger(Logger.ROOT_LOGGER_NAME).detachAppender(this); + stop(); } - static void bindStdoutAppenders(LoggerContext context, OutputStream stdout) + static boolean bindStdoutAppenders(LoggerContext context, OutputStream stdout) { OutputStream shielded = new CloseShieldOutputStream(stdout); + boolean bound = false; for (Logger logger : context.getLoggerList()) { Iterator> appenders = logger.iteratorForAppenders(); @@ -41,9 +71,43 @@ static void bindStdoutAppenders(LoggerContext context, OutputStream stdout) && ConsoleTarget.SystemOut.getName().equals(((ConsoleAppender) appender).getTarget())) { ((ConsoleAppender) appender).setOutputStream(shielded); + bound = true; } } } + return bound; + } + + @Override + public boolean isResetResistant() + { + return true; + } + + @Override + public void onStart(LoggerContext context) + { + } + + @Override + public void onReset(LoggerContext context) + { + rebindPending = stdout != null; + start(); + context.getLogger(Logger.ROOT_LOGGER_NAME).addAppender(this); + } + + @Override + public void onStop(LoggerContext context) + { + rebindPending = false; + context.getLogger(Logger.ROOT_LOGGER_NAME).detachAppender(this); + stop(); + } + + @Override + public void onLevelChange(Logger logger, Level level) + { } @Override @@ -63,6 +127,10 @@ protected void append(ILoggingEvent eventObject) { return; } + if (rebindPending && bindStdoutAppenders((LoggerContext) getContext(), stdout)) + { + rebindPending = false; + } String formatted = layout.doLayout(eventObject); logConsumer.accept(formatted); } diff --git a/runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java b/runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java index 601081f83b6..4347804b12b 100644 --- a/runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java +++ b/runelite-client/src/test/java/net/runelite/client/ui/ClientUITest.java @@ -17,6 +17,7 @@ import java.util.Collection; import java.util.concurrent.atomic.AtomicReference; import javax.inject.Provider; +import javax.swing.JPanel; import javax.swing.JScrollPane; import javax.swing.JTabbedPane; import javax.swing.JTextArea; @@ -34,6 +35,7 @@ import org.junit.Test; import org.slf4j.LoggerFactory; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -45,6 +47,7 @@ public class ClientUITest { private static final String CONFIG_GROUP = "runelite"; private static final String SIDEBAR_TAB = "clientSidebarTab"; + private static final String SIDEBAR_CLOSED = "clientSidebarClosed"; private ConfigManager configManager; private EventBus eventBus; @@ -75,8 +78,7 @@ public void tearDown() throws Exception ConsoleLogAppender consoleLogAppender = (ConsoleLogAppender) get("consoleLogAppender"); if (consoleLogAppender != null) { - root.detachAppender(consoleLogAppender); - consoleLogAppender.stop(); + consoleLogAppender.uninstall(); } if (stdoutAppender != null) { @@ -125,6 +127,32 @@ public void removingTheOpenTabWhileClosingKeepsTheSavedTab() throws Exception verify(configManager, never()).unsetConfiguration(CONFIG_GROUP, SIDEBAR_TAB); } + @Test + public void removingTheOpenTabWhileClosingDoesNotReopenAClosedSidebar() throws Exception + { + NavigationButton first = button("First", 1); + NavigationButton second = button("Second", 2); + AtomicReference sidebar = new AtomicReference<>(); + onEdt(() -> + { + installSidebar(); + sidebar.set((JTabbedPane) get("sidebar")); + clientUI.addNavigation(first); + clientUI.addNavigation(second); + clientUI.openPanel(first, true); + clientUI.openPanel(second, true); + sidebar.get().setVisible(false); + pushHistory(); + }); + + eventBus.post(new ClientShutdown()); + onEdt(() -> clientUI.removeNavigation(second)); + + verify(configManager, never()).setConfiguration(eq(CONFIG_GROUP), eq(SIDEBAR_CLOSED), any(Boolean.class)); + verify(configManager, never()).unsetConfiguration(CONFIG_GROUP, SIDEBAR_CLOSED); + assertFalse(sidebar.get().isVisible()); + } + @Test public void consoleInitializationKeepsStdoutLogLinesOutOfThePanel() throws Exception { @@ -177,7 +205,16 @@ private void installSidebar() throws Exception int index = sidebar.getSelectedIndex(); try { - set("selectedTab", index < 0 ? null : Iterables.get(entries, index)); + NavigationButton selected = index < 0 ? null : Iterables.get(entries, index); + if (selected == get("selectedTab")) + { + return; + } + set("selectedTab", selected); + if (sidebar.isVisible()) + { + pushHistory(); + } } catch (ReflectiveOperationException ex) { @@ -185,6 +222,14 @@ private void installSidebar() throws Exception } }); set("sidebar", sidebar); + set("content", new JPanel()); + } + + private void pushHistory() throws ReflectiveOperationException + { + Method pushHistory = ClientUI.class.getDeclaredMethod("pushHistory"); + pushHistory.setAccessible(true); + pushHistory.invoke(clientUI); } private static NavigationButton button(String tooltip, int priority) diff --git a/runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java b/runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java index 780cc8e6eab..1393e9d1f05 100644 --- a/runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java +++ b/runelite-client/src/test/java/net/runelite/client/ui/ConsoleLogAppenderTest.java @@ -3,8 +3,11 @@ import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.encoder.PatternLayoutEncoder; +import ch.qos.logback.classic.joran.JoranConfigurator; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.ConsoleAppender; +import ch.qos.logback.core.joran.spi.JoranException; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -18,6 +21,13 @@ public class ConsoleLogAppenderTest { + private static final String CONFIG = "" + + "" + + "%msg%n" + + "" + + "" + + ""; + private PrintStream originalOut; private LoggerContext context; @@ -97,6 +107,67 @@ public void stoppingTheLoggerContextLeavesStdoutOpen() assertEquals(1, occurrences(terminal.toString(StandardCharsets.UTF_8), "before stop")); } + @Test + public void reconfigurationKeepsTheConsoleAppenderAndStdoutBinding() throws JoranException + { + ByteArrayOutputStream terminal = new ByteArrayOutputStream(); + ByteArrayOutputStream consolePanel = new ByteArrayOutputStream(); + StringBuilder console = new StringBuilder(); + + configure(); + System.setOut(new PrintStream(new Tee(consolePanel, terminal), true, StandardCharsets.UTF_8)); + ConsoleLogAppender.install(context, terminal, console::append); + + context.reset(); + configure(); + context.getLogger(Logger.ROOT_LOGGER_NAME).info("after reset"); + + assertEquals(1, occurrences(console.toString(), "after reset")); + assertEquals(0, occurrences(consolePanel.toString(StandardCharsets.UTF_8), "after reset")); + assertEquals(1, occurrences(terminal.toString(StandardCharsets.UTF_8), "after reset")); + } + + @Test + public void lineLoggedBeforeReconfigurationEndsDoesNotLeaveStdoutUnbound() throws JoranException + { + ByteArrayOutputStream terminal = new ByteArrayOutputStream(); + ByteArrayOutputStream consolePanel = new ByteArrayOutputStream(); + StringBuilder console = new StringBuilder(); + + configure(); + System.setOut(new PrintStream(new Tee(consolePanel, terminal), true, StandardCharsets.UTF_8)); + ConsoleLogAppender.install(context, terminal, console::append); + + context.reset(); + Logger root = context.getLogger(Logger.ROOT_LOGGER_NAME); + root.info("during reset"); + configure(); + root.info("after reset"); + + assertEquals(1, occurrences(console.toString(), "during reset")); + assertEquals(1, occurrences(console.toString(), "after reset")); + assertEquals(0, occurrences(consolePanel.toString(StandardCharsets.UTF_8), "after reset")); + assertEquals(1, occurrences(terminal.toString(StandardCharsets.UTF_8), "after reset")); + } + + @Test + public void stoppingTheLoggerContextDetachesTheConsoleAppender() + { + ConsoleLogAppender appender = ConsoleLogAppender.install(context, new ByteArrayOutputStream(), line -> { }); + + context.stop(); + + assertFalse(appender.isStarted()); + assertFalse(context.getLogger(Logger.ROOT_LOGGER_NAME).isAttached(appender)); + } + + private void configure() throws JoranException + { + JoranConfigurator configurator = new JoranConfigurator(); + configurator.setContext(context); + configurator.doConfigure(new ByteArrayInputStream(CONFIG.getBytes(StandardCharsets.UTF_8))); + } + private static int occurrences(String text, String needle) { int count = 0;