From d55afba99931893f1b0811bf3e305a56438f1ad7 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Mon, 8 Apr 2019 21:57:18 +0200 Subject: [PATCH 1/3] Post change notifications on JavaFX Fixes #4817. The problem is that changes in other threads produce a "Not on FX application thread" exception. This is fixed by adding a wrapper around the list of entries. The wrapper only posts changes in the correct thread, without making a copy of the underlying list - so performance shouldn't be affected. --- src/main/java/org/jabref/gui/BasePanel.java | 2 +- .../org/jabref/gui/actions/CleanupAction.java | 11 +++++-- .../org/jabref/gui/maintable/MainTable.java | 3 +- .../org/jabref/gui/util/BindingsHelper.java | 8 +++++ .../org/jabref/gui/util/UiThreadList.java | 32 +++++++++++++++++++ 5 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 src/main/java/org/jabref/gui/util/UiThreadList.java diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java index c6501ec7da9..0fbe1e98e9d 100644 --- a/src/main/java/org/jabref/gui/BasePanel.java +++ b/src/main/java/org/jabref/gui/BasePanel.java @@ -255,7 +255,7 @@ public void output(String s) { private void setupActions() { SaveDatabaseAction saveAction = new SaveDatabaseAction(this, Globals.prefs); - CleanupAction cleanUpAction = new CleanupAction(this, Globals.prefs); + CleanupAction cleanUpAction = new CleanupAction(this, Globals.prefs, Globals.TASK_EXECUTOR); actions.put(Actions.UNDO, undoAction); actions.put(Actions.REDO, redoAction); diff --git a/src/main/java/org/jabref/gui/actions/CleanupAction.java b/src/main/java/org/jabref/gui/actions/CleanupAction.java index d79a8a62c4f..4a0f89e8b61 100644 --- a/src/main/java/org/jabref/gui/actions/CleanupAction.java +++ b/src/main/java/org/jabref/gui/actions/CleanupAction.java @@ -9,6 +9,8 @@ import org.jabref.gui.cleanup.CleanupDialog; import org.jabref.gui.undo.NamedCompound; import org.jabref.gui.undo.UndoableFieldChange; +import org.jabref.gui.util.BackgroundTask; +import org.jabref.gui.util.TaskExecutor; import org.jabref.logic.cleanup.CleanupPreset; import org.jabref.logic.cleanup.CleanupWorker; import org.jabref.logic.l10n.Localization; @@ -20,15 +22,17 @@ public class CleanupAction implements BaseAction { private final BasePanel panel; private final DialogService dialogService; + private final TaskExecutor taskExecutor; private boolean isCanceled; private int modifiedEntriesCount; private final JabRefPreferences preferences; - public CleanupAction(BasePanel panel, JabRefPreferences preferences) { + public CleanupAction(BasePanel panel, JabRefPreferences preferences, TaskExecutor taskExecutor) { this.panel = panel; this.preferences = preferences; this.dialogService = panel.frame().getDialogService(); + this.taskExecutor = taskExecutor; } @Override @@ -58,8 +62,9 @@ public void action() { preferences.setCleanupPreset(chosenPreset.get()); - this.cleanup(chosenPreset.get()); - this.showResults(); + BackgroundTask.wrap(() -> cleanup(chosenPreset.get())) + .onSuccess(result -> showResults()) + .executeWith(taskExecutor); } } diff --git a/src/main/java/org/jabref/gui/maintable/MainTable.java b/src/main/java/org/jabref/gui/maintable/MainTable.java index 892116fb337..10c9a278a2b 100644 --- a/src/main/java/org/jabref/gui/maintable/MainTable.java +++ b/src/main/java/org/jabref/gui/maintable/MainTable.java @@ -36,6 +36,7 @@ import org.jabref.gui.keyboard.KeyBindingRepository; import org.jabref.gui.undo.NamedCompound; import org.jabref.gui.undo.UndoableInsertEntry; +import org.jabref.gui.util.BindingsHelper; import org.jabref.gui.util.CustomLocalDragboard; import org.jabref.gui.util.ViewModelTableRowFactory; import org.jabref.logic.l10n.Localization; @@ -108,7 +109,7 @@ public MainTable(MainTableDataModel model, JabRefFrame frame, } this.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE); - this.setItems(model.getEntriesFilteredAndSorted()); + this.setItems(BindingsHelper.forUI(model.getEntriesFilteredAndSorted())); // Enable sorting model.getEntriesFilteredAndSorted().comparatorProperty().bind(this.comparatorProperty()); diff --git a/src/main/java/org/jabref/gui/util/BindingsHelper.java b/src/main/java/org/jabref/gui/util/BindingsHelper.java index af0fafd441b..b04b02333bb 100644 --- a/src/main/java/org/jabref/gui/util/BindingsHelper.java +++ b/src/main/java/org/jabref/gui/util/BindingsHelper.java @@ -176,6 +176,14 @@ protected String computeValue() { }; } + /** + * Returns a wrapper around the given list that posts changes on the JavaFX thread. + */ + public static ObservableList forUI(ObservableList list) { + return new UiThreadList<>(list); + } + + private static class BidirectionalBinding { private final ObservableValue propertyA; diff --git a/src/main/java/org/jabref/gui/util/UiThreadList.java b/src/main/java/org/jabref/gui/util/UiThreadList.java new file mode 100644 index 00000000000..a81a7406764 --- /dev/null +++ b/src/main/java/org/jabref/gui/util/UiThreadList.java @@ -0,0 +1,32 @@ +package org.jabref.gui.util; + +import javafx.application.Platform; +import javafx.collections.ListChangeListener; +import javafx.collections.ObservableList; +import javafx.collections.transformation.TransformationList; + +class UiThreadList extends TransformationList { + public UiThreadList(ObservableList source) { + super(source); + } + + @Override + protected void sourceChanged(ListChangeListener.Change change) { + Platform.runLater(() -> fireChange(change)); + } + + @Override + public int getSourceIndex(int index) { + return index; + } + + @Override + public T get(int index) { + return getSource().get(index); + } + + @Override + public int size() { + return getSource().size(); + } +} From cc7b3a945e546c58aa358fc306111a09f96dbb2c Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 13 Apr 2019 18:12:49 +0200 Subject: [PATCH 2/3] fix compile error --- src/main/java/org/jabref/gui/desktop/JabRefDesktop.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java index befd35ed0af..40993772e54 100644 --- a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java +++ b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java @@ -259,7 +259,7 @@ public static void openConsole(File file) throws IOException { String[] subcommands = command.split(" "); LOGGER.info("Executing command \"" + command + "\"..."); - JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Executing command \"%0\"...", commandLoggingText)); + JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Executing command \"%0\"...", command)); try { new ProcessBuilder(subcommands).start(); From c599b22495b9ab2da69a8afc2f74e583691132af Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 13 Apr 2019 18:13:45 +0200 Subject: [PATCH 3/3] fix checkstyle --- .../org/jabref/gui/util/BindingsHelper.java | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/jabref/gui/util/BindingsHelper.java b/src/main/java/org/jabref/gui/util/BindingsHelper.java index b04b02333bb..f53509c3f11 100644 --- a/src/main/java/org/jabref/gui/util/BindingsHelper.java +++ b/src/main/java/org/jabref/gui/util/BindingsHelper.java @@ -60,6 +60,7 @@ public static MappedList mapBacked(ObservableList source, Functi public static ObservableList map(ObservableValue source, Function> mapper) { PreboundBinding> binding = new PreboundBinding>(source) { + @Override protected List computeValue() { return mapper.apply(source.getValue()); @@ -103,10 +104,10 @@ public static void bindBidirectional(ObservableValue propertyA, Observ public static void bindContentBidirectional(ObservableList propertyA, ListProperty propertyB, Consumer> updateA, Consumer> updateB) { bindContentBidirectional( - propertyA, - (ObservableValue>) propertyB, - updateA, - updateB); + propertyA, + (ObservableValue>) propertyB, + updateA, + updateB); } public static void bindContentBidirectional(ObservableList propertyA, ObservableValue propertyB, Consumer updateA, Consumer> updateB) { @@ -124,10 +125,10 @@ public static void bindContentBidirectional(ListProperty listProperty, Consumer> updateB = newValueList -> property.setValue(mapToB.apply(newValueList)); bindContentBidirectional( - listProperty, - property, - updateList, - updateB); + listProperty, + property, + updateList, + updateB); } public static void bindContentBidirectional(ObservableMap propertyA, ObservableValue propertyB, Consumer updateA, Consumer> updateB) { @@ -143,14 +144,15 @@ public static void bindContentBidirectional(ObservableMap proper public static void bindContentBidirectional(ObservableMap propertyA, Property propertyB, Consumer updateA, Function, B> mapToB) { Consumer> updateB = newValueList -> propertyB.setValue(mapToB.apply(newValueList)); bindContentBidirectional( - propertyA, - propertyB, - updateA, - updateB); + propertyA, + propertyB, + updateA, + updateB); } public static ObservableValue constantOf(T value) { return new ObjectBinding() { + @Override protected T computeValue() { return value; @@ -160,6 +162,7 @@ protected T computeValue() { public static ObservableValue constantOf(boolean value) { return new BooleanBinding() { + @Override protected boolean computeValue() { return value; @@ -169,6 +172,7 @@ protected boolean computeValue() { public static ObservableValue emptyString() { return new StringBinding() { + @Override protected String computeValue() { return ""; @@ -183,7 +187,6 @@ public static ObservableList forUI(ObservableList list) { return new UiThreadList<>(list); } - private static class BidirectionalBinding { private final ObservableValue propertyA;