Merge pull request #522 from JLLeitschuh/fix/deployUIHangingGRIP

Fixes DeployUI Freezing app if too much text is displayed

Author: JLLeitschuh

ui/src/main/java/edu/wpi/grip/ui/DeployController.java

@@ -10,6 +10,7 @@
 import edu.wpi.grip.core.serialization.Project;
 import edu.wpi.grip.core.settings.ProjectSettings;
 import edu.wpi.grip.core.settings.SettingsProvider;
+import edu.wpi.grip.ui.components.LogTextArea;
 import edu.wpi.grip.ui.util.StringInMemoryFile;
 import javafx.application.Platform;
 import javafx.beans.binding.Bindings;
@@ -18,8 +19,8 @@
 import javafx.fxml.FXML;
 import javafx.scene.control.Label;
 import javafx.scene.control.ProgressIndicator;
-import javafx.scene.control.TextArea;
 import javafx.scene.control.TextField;
+import javafx.scene.control.ToggleButton;
 import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.common.StreamCopier;
 import net.schmizz.sshj.connection.channel.direct.Session;
@@ -70,11 +71,13 @@ public class DeployController {
     @FXML
     private Label status;
     @FXML
-    private TextArea console;
+    private LogTextArea console;
     @FXML
     private BooleanProperty deploying;
     @FXML
     private StringProperty command;
+    @FXML
+    private ToggleButton scrollPauseButton;
 
     @Inject
     private EventBus eventBus;
@@ -93,6 +96,10 @@ public void initialize() {
         command.bind(Bindings.concat(javaHome.textProperty(), "/bin/java ", jvmArgs.textProperty(), " -jar '",
                 deployDir.textProperty(), "/", GRIP_JAR, "' '", deployDir.textProperty(), "/", projectFile.textProperty(), "'"));
 
+        scrollPauseButton.selectedProperty().bindBidirectional(console.pausedScrollProperty());
+        console.setOnScroll(event -> {
+            console.setPausedScroll(true);
+        });
         loadSettings(settingsProvider.getProjectSettings());
     }
 
@@ -227,7 +234,7 @@ public StreamCopier.Listener file(String name, long size) {
                             return;
                         }
 
-                        Platform.runLater(() -> console.setText(console.getText() + line + "\n"));
+                        Platform.runLater(() -> console.addLineToLog(line));
                     }
                 }
             }

ui/src/main/java/edu/wpi/grip/ui/components/LogTextArea.java

@@ -0,0 +1,69 @@
+package edu.wpi.grip.ui.components;
+
+import com.google.common.annotations.VisibleForTesting;
+import javafx.beans.property.BooleanProperty;
+import javafx.beans.property.SimpleBooleanProperty;
+import javafx.scene.control.TextArea;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * A text area that is used to display a log.
+ * Implements the ability to pause scrolling and the protection of not
+ * causing the JavaFX thread to hang if the string becomes too long.
+ *
+ * @see <a href="http://stackoverflow.com/a/27747004/3708426">Original idea for this class</a>
+ */
+public class LogTextArea extends TextArea {
+    @VisibleForTesting
+    static final int MAX_STRING_LENGTH = 200000;
+    private final BooleanProperty pausedScrollProperty = new SimpleBooleanProperty(false);
+    @SuppressWarnings("PMD.AvoidStringBufferField")
+    private final StringBuilder fullLog = new StringBuilder();
+    private boolean full = false;
+
+
+    /**
+     * Adds a line to the log. This will keep track of the scroll position and maintain
+     * the scroll position if scrolling is paused.
+     *
+     * @param data The line to add to the text area.
+     */
+    public void addLineToLog(String data) {
+        checkNotNull(data, "Data cannot be null");
+        if (fullLog.length() + data.length() >= MAX_STRING_LENGTH && !full) {
+            full = true;
+            fullLog.append("[ERROR] Too much output to display. Discarding the rest.");
+        } else if (!full) {
+            fullLog.append(data + "\n");
+        } else {
+            return;
+        }
+        final double scrollPosition;
+        if (isPausedScroll()) {
+            scrollPosition = this.getScrollTop();
+        } else {
+            scrollPosition = Double.MAX_VALUE;
+        }
+        this.setText(fullLog.toString());
+        this.setScrollTop(scrollPosition);
+    }
+
+    /**
+     * @return True if the scroll has been paused
+     */
+    public final boolean isPausedScroll() {
+        return pausedScrollProperty.getValue();
+    }
+
+    /**
+     * @return A property that can be bound to prevent auto scrolling
+     */
+    public final BooleanProperty pausedScrollProperty() {
+        return pausedScrollProperty;
+    }
+
+    public final void setPausedScroll(boolean value) {
+        pausedScrollProperty.setValue(value);
+    }
+}

ui/src/main/resources/edu/wpi/grip/ui/Deploy.fxml

@@ -10,6 +10,7 @@
 <?import javafx.scene.image.Image?>
 <?import javafx.scene.image.ImageView?>
 <?import javafx.scene.layout.*?>
+<?import edu.wpi.grip.ui.components.LogTextArea?>
 <VBox styleClass="deploy-pane" maxWidth="Infinity" xmlns:fx="http://javafx.com/fxml/1"
       onKeyPressed="if (event.code == javafx.scene.input.KeyCode.ENTER) { controller.onDeploy() }"
       xmlns="http://javafx.com/javafx/null" fx:controller="edu.wpi.grip.ui.DeployController" fillWidth="true">
@@ -69,6 +70,7 @@
 
             <ButtonBar GridPane.columnIndex="0" GridPane.rowIndex="3" GridPane.columnSpan="2">
                 <buttons>
+                    <ToggleButton fx:id="scrollPauseButton" text="Pause Scroll" ButtonBar.buttonData="OTHER"/>
                     <Button fx:id="deployButton" defaultButton="true" onMouseClicked="#onDeploy" text="Deploy"
                             ButtonBar.buttonData="APPLY">
                         <fx:script>deployButton.disableProperty().bind(deploying)</fx:script>
@@ -102,6 +104,6 @@
         <Label fx:id="status"/>
     </StackPane>
 
-    <TextArea fx:id="console" editable="false" styleClass="console" prefRowCount="24" prefColumnCount="100"
+    <LogTextArea fx:id="console" editable="false" styleClass="console" prefRowCount="24" prefColumnCount="100"
               VBox.vgrow="ALWAYS"/>
 </VBox>

ui/src/test/java/edu/wpi/grip/ui/components/LogTextAreaTest.java

@@ -0,0 +1,71 @@
+package edu.wpi.grip.ui.components;
+
+
+import autovalue.shaded.org.apache.commons.lang.StringUtils;
+import javafx.scene.Scene;
+import javafx.stage.Stage;
+import org.junit.Test;
+import org.testfx.framework.junit.ApplicationTest;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+public class LogTextAreaTest extends ApplicationTest {
+    private LogTextArea logTextArea;
+
+    @Override
+    public void start(Stage stage) {
+        logTextArea = new LogTextArea();
+        stage.setScene(new Scene(logTextArea));
+        stage.show();
+    }
+
+    @Test
+    public void testAddFirstLine() {
+        final String FIRST_LINE = "First Line Of Text";
+        logTextArea.addLineToLog(FIRST_LINE);
+        assertEquals("First line wasn't added", FIRST_LINE + "\n", logTextArea.getText());
+    }
+
+    @Test
+    public void testAddTwoLines() {
+        final String FIRST_LINE = "First Line Of Text";
+        final String SECOND_LINE = "Second line of text";
+        logTextArea.addLineToLog(FIRST_LINE);
+        logTextArea.addLineToLog(SECOND_LINE);
+        assertEquals("Second line wasn't added", FIRST_LINE + "\n" + SECOND_LINE + "\n", logTextArea.getText());
+    }
+
+    @Test
+    public void testAddingReallyLongLineOfText() {
+        final String INITIAL_TEXT = StringUtils.rightPad("Initial string", LogTextArea.MAX_STRING_LENGTH - 3 , "Long\n");
+        logTextArea.addLineToLog(INITIAL_TEXT);
+        assertEquals("Text was not added to logger", INITIAL_TEXT + "\n", logTextArea.getText());
+    }
+
+    @Test
+    public void testAddingStringThatIsTooLong() {
+        final String INITIAL_TEXT = StringUtils.rightPad("Initial string", LogTextArea.MAX_STRING_LENGTH, "Long\n");
+        logTextArea.addLineToLog(INITIAL_TEXT);
+        assertNotEquals("The initial text should not have been appended as it was too long", INITIAL_TEXT + "\n", logTextArea.getText());
+    }
+
+    @Test
+    public void testAddingStringToFullLog() {
+        testAddingStringThatIsTooLong();
+        final String currentText = logTextArea.getText();
+        logTextArea.addLineToLog("More logged text");
+        assertEquals("This text should not be appended as the log is full", currentText, logTextArea.getText());
+    }
+
+    @Test
+    public void testScrollIsMaintainedWhenScrollIsPaused() {
+        logTextArea.setPausedScroll(true);
+        final double INITIAL_SCROLL = logTextArea.getScrollTop();
+        final String LONG_TEXT = StringUtils.rightPad("InitialString", 500, "Text\n");
+        logTextArea.addLineToLog(LONG_TEXT);
+        assertEquals("The log should not have scrolled", INITIAL_SCROLL, logTextArea.getScrollTop(), 0.001);
+    }
+
+
+}