From 8ba94d20c45da6daba24767d209c65d840fca188 Mon Sep 17 00:00:00 2001 From: Robin Lichtenthaeler Date: Sun, 8 Sep 2019 22:52:36 +0200 Subject: [PATCH 1/4] added uithreadaware classes --- .../LinkedFilesEditorViewModel.java | 3 +- .../uithreadaware/UiThreadChangeListener.java | 28 +++ .../util/uithreadaware/UiThreadHelper.java | 14 ++ .../UiThreadInvalidationListener.java | 28 +++ .../UiThreadListChangeListener.java | 27 +++ .../uithreadaware/UiThreadListDecorator.java | 184 ++++++++++++++++++ 6 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/jabref/gui/util/uithreadaware/UiThreadChangeListener.java create mode 100644 src/main/java/org/jabref/gui/util/uithreadaware/UiThreadHelper.java create mode 100644 src/main/java/org/jabref/gui/util/uithreadaware/UiThreadInvalidationListener.java create mode 100644 src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListChangeListener.java create mode 100644 src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListDecorator.java diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java index 471af98aa2f..b59c842df9b 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java @@ -27,6 +27,7 @@ import org.jabref.gui.util.BindingsHelper; import org.jabref.gui.util.FileDialogConfiguration; import org.jabref.gui.util.TaskExecutor; +import org.jabref.gui.util.uithreadaware.UiThreadListDecorator; import org.jabref.logic.importer.FulltextFetchers; import org.jabref.logic.integrity.FieldCheckers; import org.jabref.logic.l10n.Localization; @@ -43,7 +44,7 @@ public class LinkedFilesEditorViewModel extends AbstractEditorViewModel { - private final ListProperty files = new SimpleListProperty<>(FXCollections.observableArrayList(LinkedFileViewModel::getObservables)); + private final ListProperty files = new SimpleListProperty<>(new UiThreadListDecorator<>(FXCollections.observableArrayList(LinkedFileViewModel::getObservables))); private final BooleanProperty fulltextLookupInProgress = new SimpleBooleanProperty(false); private final DialogService dialogService; private final BibDatabaseContext databaseContext; diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadChangeListener.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadChangeListener.java new file mode 100644 index 00000000000..29d31651d5b --- /dev/null +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadChangeListener.java @@ -0,0 +1,28 @@ +package org.jabref.gui.util.uithreadaware; + +import javafx.beans.value.ChangeListener; +import javafx.beans.value.ObservableValue; + +class UiThreadChangeListener implements ChangeListener { + + private ChangeListener delegate; + + public UiThreadChangeListener(ChangeListener delegate) { + this.delegate = delegate; + } + + @Override + public void changed(ObservableValue observable, T oldValue, T newValue) { + UiThreadHelper.ensureUiThreadExecution(() -> delegate.changed(observable, oldValue, newValue)); + } + + @Override + public boolean equals(Object o) { + return delegate.equals(o); + } + + @Override + public int hashCode() { + return delegate.hashCode(); + } +} diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadHelper.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadHelper.java new file mode 100644 index 00000000000..1ea074c1bd3 --- /dev/null +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadHelper.java @@ -0,0 +1,14 @@ +package org.jabref.gui.util.uithreadaware; + +import javafx.application.Platform; + +public class UiThreadHelper { + + static void ensureUiThreadExecution(Runnable task) { + if (Platform.isFxApplicationThread()) { + task.run(); + } else { + Platform.runLater(task); + } + } +} diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadInvalidationListener.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadInvalidationListener.java new file mode 100644 index 00000000000..818c85a15a5 --- /dev/null +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadInvalidationListener.java @@ -0,0 +1,28 @@ +package org.jabref.gui.util.uithreadaware; + +import javafx.beans.InvalidationListener; +import javafx.beans.Observable; + +class UiThreadInvalidationListener implements InvalidationListener { + + private final InvalidationListener delegate; + + public UiThreadInvalidationListener(InvalidationListener delegate) { + this.delegate = delegate; + } + + @Override + public void invalidated(Observable observable) { + UiThreadHelper.ensureUiThreadExecution(() -> delegate.invalidated(observable)); + } + + @Override + public boolean equals(Object o) { + return delegate.equals(o); + } + + @Override + public int hashCode() { + return delegate.hashCode(); + } +} diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListChangeListener.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListChangeListener.java new file mode 100644 index 00000000000..609d39946ec --- /dev/null +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListChangeListener.java @@ -0,0 +1,27 @@ +package org.jabref.gui.util.uithreadaware; + +import javafx.collections.ListChangeListener; + +class UiThreadListChangeListener implements ListChangeListener { + + private final ListChangeListener delegate; + + public UiThreadListChangeListener(ListChangeListener delegate) { + this.delegate = delegate; + } + + @Override + public void onChanged(Change c) { + UiThreadHelper.ensureUiThreadExecution(() -> delegate.onChanged(c)); + } + + @Override + public boolean equals(Object o) { + return delegate.equals(o); + } + + @Override + public int hashCode() { + return delegate.hashCode(); + } +} diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListDecorator.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListDecorator.java new file mode 100644 index 00000000000..681b809afc4 --- /dev/null +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListDecorator.java @@ -0,0 +1,184 @@ +package org.jabref.gui.util.uithreadaware; + +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.ListIterator; + +import javafx.beans.InvalidationListener; +import javafx.collections.ListChangeListener; +import javafx.collections.ObservableList; + +public class UiThreadListDecorator implements ObservableList { + + private final ObservableList delegate; + + public UiThreadListDecorator(ObservableList delegate) { + this.delegate = delegate; + } + + @Override + public void addListener(ListChangeListener listener) { + delegate.addListener(new UiThreadListChangeListener(listener)); + } + + @Override + public void removeListener(ListChangeListener listener) { + delegate.removeListener(listener); + } + + @Override + public boolean addAll(E... elements) { + return delegate.addAll(elements); + } + + @Override + public boolean setAll(E... elements) { + return delegate.setAll(elements); + } + + @Override + public boolean setAll(Collection col) { + return delegate.setAll(col); + } + + @Override + public boolean removeAll(E... elements) { + return delegate.removeAll(elements); + } + + @Override + public boolean retainAll(E... elements) { + return delegate.retainAll(elements); + } + + @Override + public void remove(int from, int to) { + delegate.remove(from, to); + } + + @Override + public int size() { + return delegate.size(); + } + + @Override + public boolean isEmpty() { + return delegate.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return delegate.contains(o); + } + + @Override + public Iterator iterator() { + return delegate.iterator(); + } + + @Override + public Object[] toArray() { + return delegate.toArray(); + } + + @Override + public T[] toArray(T[] a) { + return delegate.toArray(a); + } + + @Override + public boolean add(E e) { + return delegate.add(e); + } + + @Override + public boolean remove(Object o) { + return delegate.remove(o); + } + + @Override + public boolean containsAll(Collection c) { + return delegate.containsAll(c); + } + + @Override + public boolean addAll(Collection c) { + return delegate.addAll(c); + } + + @Override + public boolean addAll(int index, Collection c) { + return delegate.addAll(index, c); + } + + @Override + public boolean removeAll(Collection c) { + return delegate.removeAll(c); + } + + @Override + public boolean retainAll(Collection c) { + return delegate.retainAll(c); + } + + @Override + public void clear() { + delegate.clear(); + } + + @Override + public E get(int index) { + return delegate.get(index); + } + + @Override + public E set(int index, E element) { + return delegate.set(index, element); + } + + @Override + public void add(int index, E element) { + delegate.add(index, element); + } + + @Override + public E remove(int index) { + return delegate.remove(index); + } + + @Override + public int indexOf(Object o) { + return delegate.indexOf(o); + } + + @Override + public int lastIndexOf(Object o) { + return delegate.lastIndexOf(o); + } + + @Override + public ListIterator listIterator() { + return delegate.listIterator(); + } + + @Override + public ListIterator listIterator(int index) { + return delegate.listIterator(index); + } + + @Override + public List subList(int fromIndex, int toIndex) { + return delegate.subList(fromIndex, toIndex); + } + + @Override + public void addListener(InvalidationListener listener) { + delegate.addListener(new UiThreadInvalidationListener(listener)); + } + + @Override + public void removeListener(InvalidationListener listener) { + delegate.removeListener(listener); + } +} From 96b9aa54c06e4e690fb400a7be2f6aa696c83128 Mon Sep 17 00:00:00 2001 From: Robin Lichtenthaeler Date: Mon, 9 Sep 2019 09:15:54 +0200 Subject: [PATCH 2/4] UiThreadHelper package-private and usage of UiThreadListDecorator only in View not in Model --- .../java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java | 5 ++++- .../jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java | 3 +-- .../org/jabref/gui/util/uithreadaware/UiThreadHelper.java | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java index ccbf0b75d54..bf3290ef391 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java @@ -34,6 +34,7 @@ import org.jabref.gui.keyboard.KeyBinding; import org.jabref.gui.util.TaskExecutor; import org.jabref.gui.util.ViewModelListCellFactory; +import org.jabref.gui.util.uithreadaware.UiThreadListDecorator; import org.jabref.logic.integrity.FieldCheckers; import org.jabref.logic.l10n.Localization; import org.jabref.model.database.BibDatabaseContext; @@ -51,6 +52,7 @@ public class LinkedFilesEditor extends HBox implements FieldEditorFX { private final DialogService dialogService; private final BibDatabaseContext databaseContext; + private final UiThreadListDecorator decoratedModelList; public LinkedFilesEditor(Field field, DialogService dialogService, BibDatabaseContext databaseContext, TaskExecutor taskExecutor, AutoCompleteSuggestionProvider suggestionProvider, FieldCheckers fieldCheckers, @@ -74,7 +76,8 @@ public LinkedFilesEditor(Field field, DialogService dialogService, BibDatabaseCo listView.setCellFactory(cellFactory); - Bindings.bindContentBidirectional(listView.itemsProperty().get(), viewModel.filesProperty()); + decoratedModelList = new UiThreadListDecorator<>(viewModel.filesProperty()); + Bindings.bindContentBidirectional(listView.itemsProperty().get(), decoratedModelList); setUpKeyBindings(); } diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java index b59c842df9b..471af98aa2f 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java @@ -27,7 +27,6 @@ import org.jabref.gui.util.BindingsHelper; import org.jabref.gui.util.FileDialogConfiguration; import org.jabref.gui.util.TaskExecutor; -import org.jabref.gui.util.uithreadaware.UiThreadListDecorator; import org.jabref.logic.importer.FulltextFetchers; import org.jabref.logic.integrity.FieldCheckers; import org.jabref.logic.l10n.Localization; @@ -44,7 +43,7 @@ public class LinkedFilesEditorViewModel extends AbstractEditorViewModel { - private final ListProperty files = new SimpleListProperty<>(new UiThreadListDecorator<>(FXCollections.observableArrayList(LinkedFileViewModel::getObservables))); + private final ListProperty files = new SimpleListProperty<>(FXCollections.observableArrayList(LinkedFileViewModel::getObservables)); private final BooleanProperty fulltextLookupInProgress = new SimpleBooleanProperty(false); private final DialogService dialogService; private final BibDatabaseContext databaseContext; diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadHelper.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadHelper.java index 1ea074c1bd3..b7590055eb9 100644 --- a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadHelper.java +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadHelper.java @@ -2,7 +2,7 @@ import javafx.application.Platform; -public class UiThreadHelper { +class UiThreadHelper { static void ensureUiThreadExecution(Runnable task) { if (Platform.isFxApplicationThread()) { From 3001c84ddd640ca5d4514bcdf7c3d1dabf73583e Mon Sep 17 00:00:00 2001 From: Robin Lichtenthaeler Date: Mon, 9 Sep 2019 10:54:16 +0200 Subject: [PATCH 3/4] added UiThreadStringProperty to be used in PersonsEditor --- .../gui/fieldeditors/LinkedFilesEditor.java | 6 +- .../gui/fieldeditors/PersonsEditor.java | 5 +- ...rator.java => UiThreadObservableList.java} | 4 +- .../uithreadaware/UiThreadStringProperty.java | 70 +++++++++++++++++++ 4 files changed, 79 insertions(+), 6 deletions(-) rename src/main/java/org/jabref/gui/util/uithreadaware/{UiThreadListDecorator.java => UiThreadObservableList.java} (96%) create mode 100644 src/main/java/org/jabref/gui/util/uithreadaware/UiThreadStringProperty.java diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java index bf3290ef391..d41c7beafcf 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java @@ -34,7 +34,7 @@ import org.jabref.gui.keyboard.KeyBinding; import org.jabref.gui.util.TaskExecutor; import org.jabref.gui.util.ViewModelListCellFactory; -import org.jabref.gui.util.uithreadaware.UiThreadListDecorator; +import org.jabref.gui.util.uithreadaware.UiThreadObservableList; import org.jabref.logic.integrity.FieldCheckers; import org.jabref.logic.l10n.Localization; import org.jabref.model.database.BibDatabaseContext; @@ -52,7 +52,7 @@ public class LinkedFilesEditor extends HBox implements FieldEditorFX { private final DialogService dialogService; private final BibDatabaseContext databaseContext; - private final UiThreadListDecorator decoratedModelList; + private final UiThreadObservableList decoratedModelList; public LinkedFilesEditor(Field field, DialogService dialogService, BibDatabaseContext databaseContext, TaskExecutor taskExecutor, AutoCompleteSuggestionProvider suggestionProvider, FieldCheckers fieldCheckers, @@ -76,7 +76,7 @@ public LinkedFilesEditor(Field field, DialogService dialogService, BibDatabaseCo listView.setCellFactory(cellFactory); - decoratedModelList = new UiThreadListDecorator<>(viewModel.filesProperty()); + decoratedModelList = new UiThreadObservableList<>(viewModel.filesProperty()); Bindings.bindContentBidirectional(listView.itemsProperty().get(), decoratedModelList); setUpKeyBindings(); } diff --git a/src/main/java/org/jabref/gui/fieldeditors/PersonsEditor.java b/src/main/java/org/jabref/gui/fieldeditors/PersonsEditor.java index 9b30d5b99dd..5faa0dbb26d 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/PersonsEditor.java +++ b/src/main/java/org/jabref/gui/fieldeditors/PersonsEditor.java @@ -7,6 +7,7 @@ import org.jabref.gui.autocompleter.AutoCompleteSuggestionProvider; import org.jabref.gui.autocompleter.AutoCompletionTextInputBinding; import org.jabref.gui.fieldeditors.contextmenu.EditorMenus; +import org.jabref.gui.util.uithreadaware.UiThreadStringProperty; import org.jabref.logic.integrity.FieldCheckers; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.Field; @@ -16,6 +17,7 @@ public class PersonsEditor extends HBox implements FieldEditorFX { private final PersonsEditorViewModel viewModel; private final TextInputControl textInput; + private final UiThreadStringProperty decoratedStringProperty; public PersonsEditor(final Field field, final AutoCompleteSuggestionProvider suggestionProvider, @@ -28,7 +30,8 @@ public PersonsEditor(final Field field, ? new EditorTextField() : new EditorTextArea(); - textInput.textProperty().bindBidirectional(viewModel.textProperty()); + decoratedStringProperty = new UiThreadStringProperty(viewModel.textProperty()); + textInput.textProperty().bindBidirectional(decoratedStringProperty); ((ContextMenuAddable) textInput).addToContextMenu(EditorMenus.getNameMenu(textInput)); this.getChildren().add(textInput); diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListDecorator.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadObservableList.java similarity index 96% rename from src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListDecorator.java rename to src/main/java/org/jabref/gui/util/uithreadaware/UiThreadObservableList.java index 681b809afc4..28cc81debc2 100644 --- a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadListDecorator.java +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadObservableList.java @@ -9,11 +9,11 @@ import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; -public class UiThreadListDecorator implements ObservableList { +public class UiThreadObservableList implements ObservableList { private final ObservableList delegate; - public UiThreadListDecorator(ObservableList delegate) { + public UiThreadObservableList(ObservableList delegate) { this.delegate = delegate; } diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadStringProperty.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadStringProperty.java new file mode 100644 index 00000000000..a0d77377edf --- /dev/null +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadStringProperty.java @@ -0,0 +1,70 @@ +package org.jabref.gui.util.uithreadaware; + +import javafx.beans.InvalidationListener; +import javafx.beans.property.StringProperty; +import javafx.beans.value.ChangeListener; +import javafx.beans.value.ObservableValue; + +public class UiThreadStringProperty extends StringProperty { + + private final StringProperty delegate; + + public UiThreadStringProperty(StringProperty delegate) { + this.delegate = delegate; + } + + @Override + public void bind(ObservableValue observable) { + delegate.bind(observable); + } + + @Override + public void unbind() { + delegate.unbind(); + } + + @Override + public boolean isBound() { + return delegate.isBound(); + } + + @Override + public Object getBean() { + return delegate.getBean(); + } + + @Override + public String getName() { + return delegate.getName(); + } + + @Override + public String get() { + return delegate.get(); + } + + @Override + public void set(String value) { + UiThreadHelper.ensureUiThreadExecution(() -> delegate.set(value)); + } + + @Override + public void addListener(ChangeListener listener) { + delegate.addListener(new UiThreadChangeListener(listener)); + } + + @Override + public void removeListener(ChangeListener listener) { + delegate.removeListener(listener); + } + + @Override + public void addListener(InvalidationListener listener) { + delegate.addListener(new UiThreadInvalidationListener(listener)); + } + + @Override + public void removeListener(InvalidationListener listener) { + delegate.removeListener(listener); + } +} From 4592dcaec881c5cce838854799de24979f6378f1 Mon Sep 17 00:00:00 2001 From: Robin Lichtenthaeler Date: Mon, 9 Sep 2019 13:59:53 +0200 Subject: [PATCH 4/4] added comments --- .../gui/util/uithreadaware/UiThreadObservableList.java | 5 +++++ .../gui/util/uithreadaware/UiThreadStringProperty.java | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadObservableList.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadObservableList.java index 28cc81debc2..17556211480 100644 --- a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadObservableList.java +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadObservableList.java @@ -9,6 +9,11 @@ import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; +/** + * This class can be used to wrap an @see ObservableList inside it. When wrapped, any Listener listening for updates to the wrapped ObservableList (for example because of a binding to it) is ensured to be notified on the JavaFX Application Thread. It should be used to implement bindings where updates come in from a background thread but should be reflected in the UI where it is necessary that changes to the UI are performed on the JavaFX Application thread. + * + * @param the type of the elements of the wrapped ObservableList. + */ public class UiThreadObservableList implements ObservableList { private final ObservableList delegate; diff --git a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadStringProperty.java b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadStringProperty.java index a0d77377edf..c271ce2df4a 100644 --- a/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadStringProperty.java +++ b/src/main/java/org/jabref/gui/util/uithreadaware/UiThreadStringProperty.java @@ -5,6 +5,10 @@ import javafx.beans.value.ChangeListener; import javafx.beans.value.ObservableValue; +/** + * This class can be used to wrap a @see StringProperty inside it. When wrapped, any Listener listening for updates to the wrapped StringProperty (for example because of a binding to it) is ensured to be notified on the JavaFX Application Thread. It should be used to implement bindings where updates come in from a background thread but should be reflected in the UI where it is necessary that changes to the UI are performed on the JavaFX Application thread. + * + */ public class UiThreadStringProperty extends StringProperty { private final StringProperty delegate; @@ -45,7 +49,7 @@ public String get() { @Override public void set(String value) { - UiThreadHelper.ensureUiThreadExecution(() -> delegate.set(value)); + delegate.set(value); } @Override