Skip to content

Commit

Permalink
Fix JavaFX thread exception when fetching new infos (#4354)
Browse files Browse the repository at this point in the history
* Fix JavaFX thread exception when fetching new infos

Fixes by reworking the SwingWorker using JavaFX BackgroundTasks.

* Fix checkstyle

* Improve code

* fix checkstyle
  • Loading branch information
tobiasdiez authored Sep 23, 2018
1 parent d8f6c5c commit 5dca3b6
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 172 deletions.
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/BasePanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public BasePanel(JabRefFrame frame, BasePanelPreferences preferences, BibDatabas

this.getDatabase().registerListener(new UpdateTimestampListener(Globals.prefs));

this.entryEditor = new EntryEditor(this, preferences.getEntryEditorPreferences(), Globals.getFileUpdateMonitor(), dialogService, externalFileTypes);
this.entryEditor = new EntryEditor(this, preferences.getEntryEditorPreferences(), Globals.getFileUpdateMonitor(), dialogService, externalFileTypes, Globals.TASK_EXECUTOR);

this.preview = new PreviewPanel(this, getBibDatabaseContext(), preferences.getKeyBindings(), preferences.getPreviewPreferences(), dialogService, externalFileTypes);
frame().getGlobalSearchBar().getSearchQueryHighlightObservable().addSearchListener(preview);
Expand Down
13 changes: 10 additions & 3 deletions src/main/java/org/jabref/gui/entryeditor/EntryEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@
import org.jabref.gui.help.HelpAction;
import org.jabref.gui.keyboard.KeyBinding;
import org.jabref.gui.menus.ChangeEntryTypeMenu;
import org.jabref.gui.mergeentries.EntryFetchAndMergeWorker;
import org.jabref.gui.mergeentries.FetchAndMergeEntry;
import org.jabref.gui.undo.CountingUndoManager;
import org.jabref.gui.util.ColorUtil;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.logic.TypedBibEntry;
import org.jabref.logic.help.HelpFile;
import org.jabref.logic.importer.EntryBasedFetcher;
Expand Down Expand Up @@ -91,14 +92,16 @@ public class EntryEditor extends BorderPane {
private final EntryEditorPreferences preferences;
private final DialogService dialogService;
private final NewDroppedFileHandler fileHandler;
private final TaskExecutor taskExecutor;

public EntryEditor(BasePanel panel, EntryEditorPreferences preferences, FileUpdateMonitor fileMonitor, DialogService dialogService, ExternalFileTypes externalFileTypes) {
public EntryEditor(BasePanel panel, EntryEditorPreferences preferences, FileUpdateMonitor fileMonitor, DialogService dialogService, ExternalFileTypes externalFileTypes, TaskExecutor taskExecutor) {
this.panel = panel;
this.databaseContext = panel.getBibDatabaseContext();
this.undoManager = panel.getUndoManager();
this.preferences = Objects.requireNonNull(preferences);
this.fileMonitor = fileMonitor;
this.dialogService = dialogService;
this.taskExecutor = taskExecutor;

fileHandler = new NewDroppedFileHandler(dialogService, databaseContext, externalFileTypes,
Globals.prefs.getFilePreferences(),
Expand Down Expand Up @@ -340,7 +343,7 @@ private void setupToolBar() {
ContextMenu fetcherMenu = new ContextMenu();
for (EntryBasedFetcher fetcher : WebFetchers.getEntryBasedFetchers(preferences.getImportFormatPreferences())) {
MenuItem fetcherMenuItem = new MenuItem(fetcher.getName());
fetcherMenuItem.setOnAction(event -> new EntryFetchAndMergeWorker(panel, getEntry(), fetcher).execute());
fetcherMenuItem.setOnAction(event -> fetchAndMerge(fetcher));
fetcherMenu.getItems().add(fetcherMenuItem);
}
fetcherButton.setOnMouseClicked(event -> fetcherMenu.show(fetcherButton, Side.RIGHT, 0, 0));
Expand All @@ -353,6 +356,10 @@ private void setupToolBar() {
generateCiteKeyButton);
}

private void fetchAndMerge(EntryBasedFetcher fetcher) {
new FetchAndMergeEntry(panel, taskExecutor).fetchAndMerge(entry, fetcher);
}

void addSearchListener(SearchQueryHighlightListener listener) {
// TODO: Highlight search text in entry editors
searchListeners.add(listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleObjectProperty;

import org.jabref.JabRefGUI;
import org.jabref.gui.DialogService;
import org.jabref.gui.autocompleter.AutoCompleteSuggestionProvider;
import org.jabref.gui.desktop.JabRefDesktop;
Expand Down Expand Up @@ -86,8 +87,8 @@ public BooleanProperty identifierLookupInProgressProperty() {
return identifierLookupInProgress;
}

public FetchAndMergeEntry fetchInformationByIdentifier(BibEntry entry) {
return new FetchAndMergeEntry(entry, fieldName);
public void fetchInformationByIdentifier(BibEntry entry) {
new FetchAndMergeEntry(JabRefGUI.getMainFrame().getCurrentBasePanel(), taskExecutor).fetchAndMerge(entry, fieldName);
}

public void lookupIdentifier(BibEntry entry) {
Expand Down

This file was deleted.

91 changes: 66 additions & 25 deletions src/main/java/org/jabref/gui/mergeentries/FetchAndMergeEntry.java
Original file line number Diff line number Diff line change
@@ -1,52 +1,93 @@
package org.jabref.gui.mergeentries;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

import org.jabref.JabRefGUI;
import org.jabref.Globals;
import org.jabref.gui.BasePanel;
import org.jabref.gui.DialogService;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.logic.importer.EntryBasedFetcher;
import org.jabref.logic.importer.IdBasedFetcher;
import org.jabref.logic.importer.WebFetchers;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.FieldName;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Class for fetching and merging information based on a specific field
*
* Class for fetching and merging bibliographic information
*/
public class FetchAndMergeEntry {

// A list of all field which are supported
public static List<String> SUPPORTED_FIELDS = Arrays.asList(FieldName.DOI, FieldName.EPRINT, FieldName.ISBN);
private static final Logger LOGGER = LoggerFactory.getLogger(FetchAndMergeEntry.class);
private final BasePanel panel;
private final DialogService dialogService;
private final TaskExecutor taskExecutor;

public FetchAndMergeEntry(BasePanel panel, TaskExecutor taskExecutor) {
this.dialogService = panel.frame().getDialogService();
this.panel = panel;
this.taskExecutor = taskExecutor;
}

/**
* Convenience constructor for a single field
*
* @param entry - BibEntry to fetch information for
* @param panel - current BasePanel
* @param field - field to get information from
*/
public FetchAndMergeEntry(BibEntry entry, BasePanel panel, String field) {
this(entry, panel, Arrays.asList(field));
public void fetchAndMerge(BibEntry entry) {
fetchAndMerge(entry, SUPPORTED_FIELDS);
}

public FetchAndMergeEntry(BibEntry entry, String field) {
this(entry, JabRefGUI.getMainFrame().getCurrentBasePanel(), field);
public void fetchAndMerge(BibEntry entry, String field) {
fetchAndMerge(entry, Collections.singletonList(field));
}

/**
* Default constructor
*
* @param entry - BibEntry to fetch information for
* @param panel - current BasePanel
* @param fields - List of fields to get information from, one at a time in given order
*/
public FetchAndMergeEntry(BibEntry entry, BasePanel panel, List<String> fields) {
public void fetchAndMerge(BibEntry entry, List<String> fields) {
for (String field : fields) {
if (entry.hasField(field)) {
new FetchAndMergeWorker(panel, entry, field).execute();
Optional<String> fieldContent = entry.getField(field);
if (fieldContent.isPresent()) {
Optional<IdBasedFetcher> fetcher = WebFetchers.getIdBasedFetcherForField(field, Globals.prefs.getImportFormatPreferences());
if (fetcher.isPresent()) {
BackgroundTask.wrap(() -> fetcher.get().performSearchById(fieldContent.get()))
.onSuccess(fetchedEntry -> {
String type = FieldName.getDisplayName(field);
if (fetchedEntry.isPresent()) {
MergeFetchedEntryDialog dialog = new MergeFetchedEntryDialog(panel, entry, fetchedEntry.get(), type);
dialog.setVisible(true);
} else {
panel.frame().setStatus(Localization.lang("Cannot get info based on given %0: %1", type, fieldContent.get()));
}
})
.onFailure(exception -> {
LOGGER.error("Error while fetching bibliographic information", exception);
dialogService.showErrorDialogAndWait(exception);
})
.executeWith(Globals.TASK_EXECUTOR);
}
} else {
panel.frame().setStatus(Localization.lang("No %0 found", FieldName.getDisplayName(field)));
dialogService.notify(Localization.lang("No %0 found", FieldName.getDisplayName(field)));
}
}
}

public void fetchAndMerge(BibEntry entry, EntryBasedFetcher fetcher) {
BackgroundTask.wrap(() -> fetcher.performSearch(entry).stream().findFirst())
.onSuccess(fetchedEntry -> {
if (fetchedEntry.isPresent()) {
MergeFetchedEntryDialog dialog = new MergeFetchedEntryDialog(panel, entry, fetchedEntry.get(), fetcher.getName());
dialog.setVisible(true);
} else {
dialogService.notify(Localization.lang("Could not find any bibliographic information."));
}
})
.onFailure(exception -> {
LOGGER.error("Error while fetching entry with " + fetcher.getName(), exception);
dialogService.showErrorDialogAndWait(Localization.lang("Error while fetching from %0", fetcher.getName()), exception);
})
.executeWith(taskExecutor);
}
}
76 changes: 0 additions & 76 deletions src/main/java/org/jabref/gui/mergeentries/FetchAndMergeWorker.java

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jabref.gui.mergeentries;

import org.jabref.Globals;
import org.jabref.gui.BasePanel;
import org.jabref.gui.DialogService;
import org.jabref.gui.actions.BaseAction;
Expand All @@ -21,7 +22,7 @@ public MergeWithFetchedEntryAction(BasePanel basePanel, DialogService dialogServ
public void action() {
if (basePanel.getMainTable().getSelectedEntries().size() == 1) {
BibEntry originalEntry = basePanel.getMainTable().getSelectedEntries().get(0);
new FetchAndMergeEntry(originalEntry, basePanel, FetchAndMergeEntry.SUPPORTED_FIELDS);
new FetchAndMergeEntry(basePanel, Globals.TASK_EXECUTOR).fetchAndMerge(originalEntry);
} else {
dialogService.showInformationDialogAndWait(Localization.lang("Merge entry with %0 information",
FieldName.orFields(FieldName.getDisplayName(FieldName.DOI),
Expand Down

0 comments on commit 5dca3b6

Please sign in to comment.