refactored code so that DialogService.notify does not need DefaultTas…

…kExecutor.runInJavaFXThread
JabRef · Apr 2, 2019 · 0444b6a · 0444b6a
1 parent 67e33e1
commit 0444b6a
Show file tree

Hide file tree

Showing 14 changed files with 58 additions and 71 deletions.
diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java
@@ -569,13 +569,12 @@ private void copyKeyAndTitle() {
     }
 
     private void openExternalFile() {
+        final List<BibEntry> selectedEntries = mainTable.getSelectedEntries();
+        if (selectedEntries.size() != 1) {
+            output(Localization.lang("This operation requires exactly one item to be selected."));
+            return;
+        }
         JabRefExecutorService.INSTANCE.execute(() -> {
-            final List<BibEntry> selectedEntries = mainTable.getSelectedEntries();
-            if (selectedEntries.size() != 1) {
-                output(Localization.lang("This operation requires exactly one item to be selected."));
-                return;
-            }
-
             final BibEntry entry = selectedEntries.get(0);
             if (!entry.hasField(FieldName.FILE)) {
                 // no bibtex field

diff --git a/src/main/java/org/jabref/gui/FXDialogService.java b/src/main/java/org/jabref/gui/FXDialogService.java
@@ -33,7 +33,6 @@
 import javafx.util.Duration;
 
 import org.jabref.gui.icon.IconTheme;
-import org.jabref.gui.util.DefaultTaskExecutor;
 import org.jabref.gui.util.DirectoryDialogConfiguration;
 import org.jabref.gui.util.FileDialogConfiguration;
 import org.jabref.gui.util.ZipFileChooser;
@@ -271,7 +270,7 @@ public <V> void showProgressDialogAndWait(String title, String content, Task<V>
     @Override
     public void notify(String message) {
         LOGGER.info(message);
-        DefaultTaskExecutor.runInJavaFXThread(() -> statusLine.fireEvent(new SnackbarEvent(new JFXSnackbarLayout(message), TOAST_MESSAGE_DISPLAY_TIME, null)));
+        statusLine.fireEvent(new SnackbarEvent(new JFXSnackbarLayout(message), TOAST_MESSAGE_DISPLAY_TIME, null));
     }
 
     @Override

diff --git a/src/main/java/org/jabref/gui/actions/CleanupAction.java b/src/main/java/org/jabref/gui/actions/CleanupAction.java
@@ -71,7 +71,7 @@ public void init() {
             isCanceled = true;
             return;
         }
-        panel.output(Localization.lang("Doing a cleanup for %0 entries...",
+        dialogService.notify(Localization.lang("Doing a cleanup for %0 entries...",
                 Integer.toString(panel.getSelectedEntries().size())));
     }
 
@@ -115,7 +115,7 @@ private void showResults() {
                 message = Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount));
                 break;
         }
-        panel.output(message);
+        dialogService.notify(message);
     }
 
     private void cleanup(CleanupPreset cleanupPreset) {

diff --git a/src/main/java/org/jabref/gui/actions/GenerateBibtexKeyAction.java b/src/main/java/org/jabref/gui/actions/GenerateBibtexKeyAction.java
@@ -33,7 +33,7 @@ public void init() {
                                                    Localization.lang("First select the entries you want keys to be generated for."));
             return;
         }
-        basePanel.output(formatOutputMessage(Localization.lang("Generating BibTeX key for"), entries.size()));
+        dialogService.notify(formatOutputMessage(Localization.lang("Generating BibTeX key for"), entries.size()));
     }
 
     public static boolean confirmOverwriteKeys(DialogService dialogService) {
@@ -87,7 +87,7 @@ private void generateKeys() {
         }
 
         basePanel.markBaseChanged();
-        basePanel.output(formatOutputMessage(Localization.lang("Generated BibTeX key for"), entries.size()));
+        dialogService.notify(formatOutputMessage(Localization.lang("Generated BibTeX key for"), entries.size()));
     }
 
     private String formatOutputMessage(String start, int count) {

diff --git a/src/main/java/org/jabref/gui/actions/LookupIdentifierAction.java b/src/main/java/org/jabref/gui/actions/LookupIdentifierAction.java
@@ -12,6 +12,7 @@
 import org.jabref.gui.undo.NamedCompound;
 import org.jabref.gui.undo.UndoableFieldChange;
 import org.jabref.gui.util.BackgroundTask;
+import org.jabref.gui.util.DefaultTaskExecutor;
 import org.jabref.logic.importer.FetcherException;
 import org.jabref.logic.importer.IdFetcher;
 import org.jabref.logic.l10n.Localization;
@@ -84,8 +85,9 @@ private String lookupIdentifiers() {
         int foundCount = 0;
         for (BibEntry bibEntry : bibEntries) {
             count++;
-            frame.getDialogService().notify(Localization.lang("Looking up %0... - entry %1 out of %2 - found %3",
-                    fetcher.getIdentifierName(), Integer.toString(count), totalCount, Integer.toString(foundCount)));
+            final String statusMessage = Localization.lang("Looking up %0... - entry %1 out of %2 - found %3",
+                    fetcher.getIdentifierName(), Integer.toString(count), totalCount, Integer.toString(foundCount));
+            DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(statusMessage));
             Optional<T> identifier = Optional.empty();
             try {
                 identifier = fetcher.findIdentifier(bibEntry);
@@ -97,8 +99,9 @@ private String lookupIdentifiers() {
                 if (fieldChange.isPresent()) {
                     namedCompound.addEdit(new UndoableFieldChange(fieldChange.get()));
                     foundCount++;
-                    frame.getDialogService().notify(Localization.lang("Looking up %0... - entry %1 out of %2 - found %3",
-                            Integer.toString(count), totalCount, Integer.toString(foundCount)));
+                    final String nextStatusMessage = Localization.lang("Looking up %0... - entry %1 out of %2 - found %3",
+                            fetcher.getIdentifierName(), Integer.toString(count), totalCount, Integer.toString(foundCount));
+                    DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(nextStatusMessage));
                 }
             }
         }

diff --git a/src/main/java/org/jabref/gui/actions/WriteXMPAction.java b/src/main/java/org/jabref/gui/actions/WriteXMPAction.java
@@ -92,7 +92,7 @@ public void init() {
         }
         optionsDialog.open();
 
-        basePanel.output(Localization.lang("Writing XMP-metadata..."));
+        dialogService.notify(Localization.lang("Writing XMP-metadata..."));
     }
 
     private void writeXMP() {
@@ -162,7 +162,7 @@ private void writeXMP() {
             return;
         }
 
-        basePanel.output(Localization.lang("Finished writing XMP for %0 file (%1 skipped, %2 errors).",
+        dialogService.notify(Localization.lang("Finished writing XMP for %0 file (%1 skipped, %2 errors).",
                 String.valueOf(entriesChanged), String.valueOf(skipped), String.valueOf(errors)));
     }
 

diff --git a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java
@@ -251,7 +251,6 @@ public static void openConsole(File file) throws IOException {
                                                   Localization.lang("Error occured while executing the command \"%0\".", commandLoggingText),
                                                   Localization.lang("Open console") + " - " + Localization.lang("Error"),
                                                   JOptionPane.ERROR_MESSAGE);
-                    JabRefGUI.getMainFrame().getDialogService().notify(null);
                 }
             }
         }

diff --git a/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java b/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java
@@ -53,7 +53,7 @@ public void execute() {
         }
 
         if (panel.getSelectedEntries().isEmpty()) {
-            panel.output(Localization.lang("This operation requires one or more entries to be selected."));
+            dialogService.notify(Localization.lang("This operation requires one or more entries to be selected."));
             return;
         }
 
@@ -123,7 +123,7 @@ private void setContentToClipboard(String content) {
         clipboardContent.putRtf(content);
         Globals.clipboardManager.setContent(clipboardContent);
 
-        panel.output(Localization.lang("Entries exported to clipboard") + ": " + entries.size());
+        dialogService.notify(Localization.lang("Entries exported to clipboard") + ": " + entries.size());
 
     }
 

diff --git a/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java b/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java
@@ -2,8 +2,6 @@
 
 import java.net.URL;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
@@ -42,12 +40,6 @@ public FindFullTextAction(BasePanel basePanel) {
 
     @Override
     public void execute() {
-        BackgroundTask.wrap(this::findFullTexts)
-                      .onSuccess(this::downloadFullTexts)
-                      .executeWith(Globals.TASK_EXECUTOR);
-    }
-
-    private Map<Optional<URL>, BibEntry> findFullTexts() {
         if (!basePanel.getSelectedEntries().isEmpty()) {
             basePanel.output(Localization.lang("Looking for full text document..."));
         } else {
@@ -68,21 +60,24 @@ private Map<Optional<URL>, BibEntry> findFullTexts() {
 
             if (!confirmDownload) {
                 basePanel.output(Localization.lang("Operation canceled."));
-                return null;
+                return;
             }
         }
+        BackgroundTask.wrap(this::findFullTexts)
+                      .onSuccess(this::downloadFullTexts)
+                      .executeWith(Globals.TASK_EXECUTOR);
+    }
 
+    private Map<Optional<URL>, BibEntry> findFullTexts() {
         Map<Optional<URL>, BibEntry> downloads = new ConcurrentHashMap<>();
         for (BibEntry entry : basePanel.getSelectedEntries()) {
             FulltextFetchers fetchers = new FulltextFetchers(Globals.prefs.getImportFormatPreferences());
             downloads.put(fetchers.findFullTextPDF(entry), entry);
         }
-
         return downloads;
     }
 
     private void downloadFullTexts(Map<Optional<URL>, BibEntry> downloads) {
-        List<Optional<URL>> finishedTasks = new ArrayList<>();
         for (Map.Entry<Optional<URL>, BibEntry> download : downloads.entrySet()) {
             BibEntry entry = download.getValue();
             Optional<URL> result = download.getKey();
@@ -94,20 +89,15 @@ private void downloadFullTexts(Map<Optional<URL>, BibEntry> downloads) {
                     dialogService.showErrorDialogAndWait(Localization.lang("Directory not found"),
                             Localization.lang("Main file directory not set!") + " " + Localization.lang("Preferences")
                                     + " -> " + Localization.lang("File"));
-
                     return;
                 }
-
-                //Download full text
+                //Download and link full text
                 addLinkedFileFromURL(result.get(), entry);
+
             } else {
                 dialogService.notify(Localization.lang("No full text document found for entry %0.",
                         entry.getCiteKeyOptional().orElse(Localization.lang("undefined"))));
             }
-            finishedTasks.add(result);
-        }
-        for (Optional<URL> result : finishedTasks) {
-            downloads.remove(result);
         }
     }
 
@@ -119,7 +109,6 @@ private void downloadFullTexts(Map<Optional<URL>, BibEntry> downloads) {
      * @param entry the entry "value"
      */
     private void addLinkedFileFromURL(URL url, BibEntry entry) {
-
         LinkedFile newLinkedFile = new LinkedFile(url, "");
 
         if (!entry.getFiles().contains(newLinkedFile)) {

diff --git a/src/main/java/org/jabref/gui/filelist/AttachFileAction.java b/src/main/java/org/jabref/gui/filelist/AttachFileAction.java
@@ -28,7 +28,7 @@ public AttachFileAction(BasePanel panel, DialogService dialogService) {
     @Override
     public void execute() {
         if (panel.getSelectedEntries().size() != 1) {
-            panel.output(Localization.lang("This operation requires exactly one item to be selected."));
+            dialogService.notify(Localization.lang("This operation requires exactly one item to be selected."));
             return;
         }
         BibEntry entry = panel.getSelectedEntries().get(0);

diff --git a/src/main/java/org/jabref/gui/importer/ImportAction.java b/src/main/java/org/jabref/gui/importer/ImportAction.java
@@ -15,6 +15,7 @@
 import org.jabref.gui.DialogService;
 import org.jabref.gui.JabRefFrame;
 import org.jabref.gui.util.BackgroundTask;
+import org.jabref.gui.util.DefaultTaskExecutor;
 import org.jabref.gui.util.TaskExecutor;
 import org.jabref.logic.importer.ImportException;
 import org.jabref.logic.importer.ImportFormatReader;
@@ -95,11 +96,11 @@ private List<ImportFormatReader.UnknownFormatImport> doImport(List<Path> files)
             try {
                 if (!importer.isPresent()) {
                     // Unknown format:
-                    frame.getDialogService().notify(Localization.lang("Importing in unknown format") + "...");
+                    DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(Localization.lang("Importing in unknown format") + "..."));
                     // This import method never throws an IOException:
                     imports.add(Globals.IMPORT_FORMAT_READER.importUnknownFormat(filename, Globals.getFileUpdateMonitor()));
                 } else {
-                    frame.getDialogService().notify(Localization.lang("Importing in %0 format", importer.get().getName()) + "...");
+                    DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(Localization.lang("Importing in %0 format", importer.get().getName()) + "..."));
                     // Specific importer:
                     ParserResult pr = importer.get().importDatabase(filename, Globals.prefs.getDefaultEncoding());
                     imports.add(new ImportFormatReader.UnknownFormatImport(importer.get().getName(), pr));

diff --git a/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java
@@ -9,7 +9,6 @@
 import javax.swing.undo.CompoundEdit;
 
 import org.jabref.Globals;
-import org.jabref.JabRefExecutorService;
 import org.jabref.gui.BasePanel;
 import org.jabref.gui.DialogService;
 import org.jabref.gui.JabRefFrame;
@@ -18,6 +17,7 @@
 import org.jabref.gui.undo.NamedCompound;
 import org.jabref.gui.undo.UndoableInsertEntry;
 import org.jabref.gui.undo.UndoableInsertString;
+import org.jabref.gui.util.BackgroundTask;
 import org.jabref.gui.util.FileDialogConfiguration;
 import org.jabref.logic.importer.OpenDatabase;
 import org.jabref.logic.importer.ParserResult;
@@ -159,32 +159,31 @@ public void action() {
             }
             filesToOpen.addAll(chosen);
 
-            // Run the actual open in a thread to prevent the program locking until the file is loaded.
-            JabRefExecutorService.INSTANCE.execute(
-                    () -> openIt(dialog.importEntries(), dialog.importStrings(), dialog.importGroups(), dialog.importSelectorWords()));
-        }
-    }
+            if (filesToOpen.isEmpty()) {
+                return;
+            }
 
-    private void openIt(boolean importEntries, boolean importStrings, boolean importGroups,
-                        boolean importSelectorWords) {
-        if (filesToOpen.isEmpty()) {
-            return;
-        }
-        for (Path file : filesToOpen) {
-            try {
-                Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, file.getParent().toString());
-                // Should this be done _after_ we know it was successfully opened?
-                ParserResult parserResult = OpenDatabase.loadDatabase(file.toFile(),
-                        Globals.prefs.getImportFormatPreferences(), Globals.getFileUpdateMonitor());
-                AppendDatabaseAction.mergeFromBibtex(panel, parserResult, importEntries, importStrings, importGroups,
-                        importSelectorWords);
-                panel.output(Localization.lang("Imported from library") + " '" + file + "'");
-            } catch (IOException | KeyCollisionException ex) {
-                LOGGER.warn("Could not open database", ex);
-
-                dialogService.showErrorDialogAndWait(Localization.lang("Open library"), ex);
+            for (Path file : filesToOpen) {
+                // Run the actual open in a thread to prevent the program locking until the file is loaded.
+                BackgroundTask.wrap(() -> openIt(file, dialog.importEntries(), dialog.importStrings(), dialog.importGroups(), dialog.importSelectorWords()))
+                              .onSuccess(fileName -> { dialogService.notify(Localization.lang("Imported from library") + " '" + fileName + "'");})
+                              .onFailure(exception -> {
+                                  LOGGER.warn("Could not open database", exception);
+                                  dialogService.showErrorDialogAndWait(Localization.lang("Open library"), exception);})
+                              .executeWith(Globals.TASK_EXECUTOR);;
             }
         }
     }
 
+    private String openIt(Path file, boolean importEntries, boolean importStrings, boolean importGroups,
+                        boolean importSelectorWords) throws IOException, KeyCollisionException {
+            Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, file.getParent().toString());
+            // Should this be done _after_ we know it was successfully opened?
+            ParserResult parserResult = OpenDatabase.loadDatabase(file.toFile(),
+                    Globals.prefs.getImportFormatPreferences(), Globals.getFileUpdateMonitor());
+            AppendDatabaseAction.mergeFromBibtex(panel, parserResult, importEntries, importStrings, importGroups,
+                    importSelectorWords);
+            return file.toString();
+    }
+
 }
diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java
@@ -37,15 +37,14 @@ public AbbreviateAction(BasePanel panel, boolean iso) {
 
     @Override
     public void action() {
+        panel.output(Localization.lang("Abbreviating..."));
         BackgroundTask.wrap(this::abbreviate)
                       .onSuccess(panel::output)
                       .executeWith(Globals.TASK_EXECUTOR);
 
     }
 
     private String abbreviate() {
-        panel.output(Localization.lang("Abbreviating..."));
-
         List<BibEntry> entries = panel.getSelectedEntries();
         UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(
                 Globals.journalAbbreviationLoader.getRepository(Globals.prefs.getJournalAbbreviationPreferences()),

diff --git a/src/main/java/org/jabref/gui/journals/UnabbreviateAction.java b/src/main/java/org/jabref/gui/journals/UnabbreviateAction.java
@@ -24,14 +24,13 @@ public UnabbreviateAction(BasePanel panel) {
 
     @Override
     public void action() {
+        panel.output(Localization.lang("Unabbreviating..."));
         BackgroundTask.wrap(this::unabbreviate)
                       .onSuccess(panel::output)
                       .executeWith(Globals.TASK_EXECUTOR);
     }
 
     private String unabbreviate() {
-        panel.output(Localization.lang("Unabbreviating..."));
-
         List<BibEntry> entries = panel.getSelectedEntries(); // never null
 
         UndoableUnabbreviator undoableAbbreviator = new UndoableUnabbreviator(Globals.journalAbbreviationLoader