Skip to content

Commit 7e40f6a

Browse files
committed
Fixes DeployUI Freezing app if too much text is displayed
The JavaFX thread would hang when the string being displayed in the deploy UI became too long. This ensures that if the string becomes too long it will stop displaying more data.
1 parent 88b8114 commit 7e40f6a

File tree

4 files changed

+153
-4
lines changed

4 files changed

+153
-4
lines changed

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import edu.wpi.grip.core.serialization.Project;
1111
import edu.wpi.grip.core.settings.ProjectSettings;
1212
import edu.wpi.grip.core.settings.SettingsProvider;
13+
import edu.wpi.grip.ui.components.LogTextArea;
1314
import edu.wpi.grip.ui.util.StringInMemoryFile;
1415
import javafx.application.Platform;
1516
import javafx.beans.binding.Bindings;
@@ -18,8 +19,8 @@
1819
import javafx.fxml.FXML;
1920
import javafx.scene.control.Label;
2021
import javafx.scene.control.ProgressIndicator;
21-
import javafx.scene.control.TextArea;
2222
import javafx.scene.control.TextField;
23+
import javafx.scene.control.ToggleButton;
2324
import net.schmizz.sshj.SSHClient;
2425
import net.schmizz.sshj.common.StreamCopier;
2526
import net.schmizz.sshj.connection.channel.direct.Session;
@@ -70,11 +71,13 @@ public class DeployController {
7071
@FXML
7172
private Label status;
7273
@FXML
73-
private TextArea console;
74+
private LogTextArea console;
7475
@FXML
7576
private BooleanProperty deploying;
7677
@FXML
7778
private StringProperty command;
79+
@FXML
80+
private ToggleButton scrollPauseButton;
7881

7982
@Inject
8083
private EventBus eventBus;
@@ -93,6 +96,10 @@ public void initialize() {
9396
command.bind(Bindings.concat(javaHome.textProperty(), "/bin/java ", jvmArgs.textProperty(), " -jar '",
9497
deployDir.textProperty(), "/", GRIP_JAR, "' '", deployDir.textProperty(), "/", projectFile.textProperty(), "'"));
9598

99+
scrollPauseButton.selectedProperty().bindBidirectional(console.pausedScrollProperty());
100+
console.setOnScroll(event -> {
101+
console.setPausedScroll(true);
102+
});
96103
loadSettings(settingsProvider.getProjectSettings());
97104
}
98105

@@ -227,7 +234,7 @@ public StreamCopier.Listener file(String name, long size) {
227234
return;
228235
}
229236

230-
Platform.runLater(() -> console.setText(console.getText() + line + "\n"));
237+
Platform.runLater(() -> console.addLineToLog(line));
231238
}
232239
}
233240
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package edu.wpi.grip.ui.components;
2+
3+
import com.google.common.annotations.VisibleForTesting;
4+
import javafx.beans.property.BooleanProperty;
5+
import javafx.beans.property.SimpleBooleanProperty;
6+
import javafx.scene.control.TextArea;
7+
8+
import static com.google.common.base.Preconditions.checkNotNull;
9+
10+
/**
11+
* A text area that is used to display a log.
12+
* Implements the ability to pause scrolling and the protection of not
13+
* causing the JavaFX thread to hang if the string becomes too long.
14+
*
15+
* @see <a href="http://stackoverflow.com/a/27747004/3708426">Original idea for this class</a>
16+
*/
17+
public class LogTextArea extends TextArea {
18+
@VisibleForTesting
19+
static final int MAX_STRING_LENGTH = 200000;
20+
private final BooleanProperty pausedScrollProperty = new SimpleBooleanProperty(false);
21+
@SuppressWarnings("PMD.AvoidStringBufferField")
22+
private final StringBuilder fullLog = new StringBuilder();
23+
private boolean full = false;
24+
25+
26+
/**
27+
* Adds a line to the log. This will keep track of the scroll position and maintain
28+
* the scroll position if scrolling is paused.
29+
*
30+
* @param data The line to add to the text area.
31+
*/
32+
public void addLineToLog(String data) {
33+
checkNotNull(data, "Data cannot be null");
34+
if (fullLog.length() + data.length() >= MAX_STRING_LENGTH && !full) {
35+
full = true;
36+
fullLog.append("[ERROR] Too much output to display. Discarding the rest.");
37+
} else if (!full) {
38+
fullLog.append(data + "\n");
39+
} else {
40+
return;
41+
}
42+
final double scrollPosition;
43+
if (isPausedScroll()) {
44+
scrollPosition = this.getScrollTop();
45+
} else {
46+
scrollPosition = Double.MAX_VALUE;
47+
}
48+
this.setText(fullLog.toString());
49+
this.setScrollTop(scrollPosition);
50+
}
51+
52+
/**
53+
* @return True if the scroll has been paused
54+
*/
55+
public final boolean isPausedScroll() {
56+
return pausedScrollProperty.getValue();
57+
}
58+
59+
/**
60+
* @return A property that can be bound to prevent auto scrolling
61+
*/
62+
public final BooleanProperty pausedScrollProperty() {
63+
return pausedScrollProperty;
64+
}
65+
66+
public final void setPausedScroll(boolean value) {
67+
pausedScrollProperty.setValue(value);
68+
}
69+
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
<?import javafx.scene.image.Image?>
1111
<?import javafx.scene.image.ImageView?>
1212
<?import javafx.scene.layout.*?>
13+
<?import edu.wpi.grip.ui.components.LogTextArea?>
1314
<VBox styleClass="deploy-pane" maxWidth="Infinity" xmlns:fx="http://javafx.com/fxml/1"
1415
onKeyPressed="if (event.code == javafx.scene.input.KeyCode.ENTER) { controller.onDeploy() }"
1516
xmlns="http://javafx.com/javafx/null" fx:controller="edu.wpi.grip.ui.DeployController" fillWidth="true">
@@ -69,6 +70,7 @@
6970

7071
<ButtonBar GridPane.columnIndex="0" GridPane.rowIndex="3" GridPane.columnSpan="2">
7172
<buttons>
73+
<ToggleButton fx:id="scrollPauseButton" text="Pause Scroll" ButtonBar.buttonData="OTHER"/>
7274
<Button fx:id="deployButton" defaultButton="true" onMouseClicked="#onDeploy" text="Deploy"
7375
ButtonBar.buttonData="APPLY">
7476
<fx:script>deployButton.disableProperty().bind(deploying)</fx:script>
@@ -102,6 +104,6 @@
102104
<Label fx:id="status"/>
103105
</StackPane>
104106

105-
<TextArea fx:id="console" editable="false" styleClass="console" prefRowCount="24" prefColumnCount="100"
107+
<LogTextArea fx:id="console" editable="false" styleClass="console" prefRowCount="24" prefColumnCount="100"
106108
VBox.vgrow="ALWAYS"/>
107109
</VBox>
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package edu.wpi.grip.ui.components;
2+
3+
4+
import autovalue.shaded.org.apache.commons.lang.StringUtils;
5+
import javafx.scene.Scene;
6+
import javafx.stage.Stage;
7+
import org.junit.Test;
8+
import org.testfx.framework.junit.ApplicationTest;
9+
10+
import static org.junit.Assert.assertEquals;
11+
import static org.junit.Assert.assertNotEquals;
12+
13+
public class LogTextAreaTest extends ApplicationTest {
14+
private LogTextArea logTextArea;
15+
16+
@Override
17+
public void start(Stage stage) {
18+
logTextArea = new LogTextArea();
19+
stage.setScene(new Scene(logTextArea));
20+
stage.show();
21+
}
22+
23+
@Test
24+
public void testAddFirstLine() {
25+
final String FIRST_LINE = "First Line Of Text";
26+
logTextArea.addLineToLog(FIRST_LINE);
27+
assertEquals("First line wasn't added", FIRST_LINE + "\n", logTextArea.getText());
28+
}
29+
30+
@Test
31+
public void testAddTwoLines() {
32+
final String FIRST_LINE = "First Line Of Text";
33+
final String SECOND_LINE = "Second line of text";
34+
logTextArea.addLineToLog(FIRST_LINE);
35+
logTextArea.addLineToLog(SECOND_LINE);
36+
assertEquals("Second line wasn't added", FIRST_LINE + "\n" + SECOND_LINE + "\n", logTextArea.getText());
37+
}
38+
39+
@Test
40+
public void testAddingReallyLongLineOfText() {
41+
final String INITIAL_TEXT = StringUtils.rightPad("Initial string", LogTextArea.MAX_STRING_LENGTH - 3 , "Long\n");
42+
logTextArea.addLineToLog(INITIAL_TEXT);
43+
assertEquals("Text was not added to logger", INITIAL_TEXT + "\n", logTextArea.getText());
44+
}
45+
46+
@Test
47+
public void testAddingStringThatIsTooLong() {
48+
final String INITIAL_TEXT = StringUtils.rightPad("Initial string", LogTextArea.MAX_STRING_LENGTH, "Long\n");
49+
logTextArea.addLineToLog(INITIAL_TEXT);
50+
assertNotEquals("The initial text should not have been appended as it was too long", INITIAL_TEXT + "\n", logTextArea.getText());
51+
}
52+
53+
@Test
54+
public void testAddingStringToFullLog() {
55+
testAddingStringThatIsTooLong();
56+
final String currentText = logTextArea.getText();
57+
logTextArea.addLineToLog("More logged text");
58+
assertEquals("This text should not be appended as the log is full", currentText, logTextArea.getText());
59+
}
60+
61+
@Test
62+
public void testScrollIsMaintainedWhenScrollIsPaused() {
63+
logTextArea.setPausedScroll(true);
64+
final double INITIAL_SCROLL = logTextArea.getScrollTop();
65+
final String LONG_TEXT = StringUtils.rightPad("InitialString", 500, "Text\n");
66+
logTextArea.addLineToLog(LONG_TEXT);
67+
assertEquals("The log should not have scrolled", INITIAL_SCROLL, logTextArea.getScrollTop(), 0.001);
68+
}
69+
70+
71+
}

0 commit comments

Comments
 (0)