Attach each full text document as soon as its search finishes

CHANGELOG.md

@@ -18,6 +18,7 @@
 ### Fixed
 
 - We fixed an issue where the AI chat lost its scroll position when switching back to an entry. [#17172](https://github.com/JabRef/jabref/pull/17172)
+- We fixed an issue where full text documents for several selected entries were attached only after all searches finished. [koppor#772](https://github.com/JabRef/jabref-koppor/pull/772)
 
 ### Removed
 

docs/requirements/fetchers.md

@@ -51,7 +51,7 @@ For entries that contain a ScholarAPI identifier and have a PDF available, JabRe
 ## Full text search runs in the background
 `req~fetchers.fulltext-background-search~1`
 
-The search for full text documents runs as a background task shown in the status bar with progress and a cancel option, so JabRef stays usable while it runs. Its results are applied to the library the entries were selected in, and are discarded if that library was closed meanwhile.
+The search for full text documents runs as a background task shown in the status bar with progress and a cancel option, so JabRef stays usable while it runs. Each entry's result is applied as soon as its search finishes, without waiting for the remaining entries. Results are applied to the library the entries were selected in, and are discarded if that library was closed meanwhile.
 
 Needs: impl
 

jabgui/src/main/java/org/jabref/gui/externalfiles/DownloadFullTextAction.java

@@ -1,8 +1,8 @@
 package org.jabref.gui.externalfiles;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
+import java.util.function.Consumer;
 import java.util.function.Function;
 
 import org.jabref.gui.DialogService;
@@ -38,6 +38,7 @@ public class DownloadFullTextAction extends SimpleCommand {
     private final GuiPreferences preferences;
     private final UiTaskExecutor taskExecutor;
     private final Function<BibEntry, Optional<FetcherResult>> fullTextFinder;
+    private final Consumer<Runnable> uiThread;
 
     public DownloadFullTextAction(DialogService dialogService,
                                   StateManager stateManager,
@@ -47,19 +48,22 @@ public DownloadFullTextAction(DialogService dialogService,
                 stateManager,
                 preferences,
                 taskExecutor,
-                entry -> new FulltextFetchers(preferences.getImportFormatPreferences(), preferences.getImporterPreferences()).findFullTextPDF(entry));
+                entry -> new FulltextFetchers(preferences.getImportFormatPreferences(), preferences.getImporterPreferences()).findFullTextPDF(entry),
+                UiTaskExecutor::runInJavaFXThread);
     }
 
     DownloadFullTextAction(DialogService dialogService,
                            StateManager stateManager,
                            GuiPreferences preferences,
                            UiTaskExecutor taskExecutor,
-                           Function<BibEntry, Optional<FetcherResult>> fullTextFinder) {
+                           Function<BibEntry, Optional<FetcherResult>> fullTextFinder,
+                           Consumer<Runnable> uiThread) {
         this.dialogService = dialogService;
         this.stateManager = stateManager;
         this.preferences = preferences;
         this.taskExecutor = taskExecutor;
         this.fullTextFinder = fullTextFinder;
+        this.uiThread = uiThread;
 
         this.executable.bind(ActionHelper.needsEntriesSelected(stateManager));
     }
@@ -92,53 +96,51 @@ private void execute(BibDatabaseContext databaseContext) {
             }
         }
 
-        BackgroundTask<List<EntryDownload>> findFullTextsTask = new BackgroundTask<>() {
+        BackgroundTask<Void> findFullTextsTask = new BackgroundTask<>() {
             @Override
-            public List<EntryDownload> call() {
-                List<EntryDownload> downloads = new ArrayList<>(entries.size());
+            public Void call() {
                 int count = 0;
                 for (BibEntry entry : entries) {
                     if (isCancelled()) {
                         break;
                     }
 
                     BibEntry lookupSnapshot = new BibEntry(entry);
-                    downloads.add(new EntryDownload(entry, lookupSnapshot, fullTextFinder.apply(lookupSnapshot)));
+                    EntryDownload download = new EntryDownload(entry, lookupSnapshot, fullTextFinder.apply(lookupSnapshot));
+                    // Attach right away: a lookup can take minutes, so the user should not wait for the whole selection
+                    uiThread.accept(() -> downloadFullText(download, databaseContext));
                     updateProgress(++count, entries.size());
                     updateMessage(Localization.lang("%0/%1 entries", count, entries.size()));
                 }
-                return downloads;
+                return null;
             }
         };
 
         findFullTextsTask.setTitle(Localization.lang("Download full text documents"))
                          .withInitialMessage(Localization.lang("Looking for full text document..."))
                          .showToUser(true)
-                         .onSuccess(downloads -> downloadFullTexts(downloads, databaseContext))
                          .executeWith(taskExecutor);
     }
 
-    private void downloadFullTexts(List<EntryDownload> downloads, BibDatabaseContext databaseContext) {
+    private void downloadFullText(EntryDownload download, BibDatabaseContext databaseContext) {
         if (!stateManager.getOpenDatabases().contains(databaseContext)) {
-            LOGGER.debug("Library closed before the full text search finished; skipping downloads.");
+            LOGGER.debug("Library closed before the full text search finished; skipping download.");
             return;
         }
 
-        for (EntryDownload download : downloads) {
-            BibEntry entry = download.entry();
-            if (!databaseContext.getDatabase().getEntries().contains(entry)) {
-                continue;
-            }
-            if (!entry.equals(download.lookupSnapshot())) {
-                LOGGER.debug("Entry changed during full text search; skipping download.");
-                continue;
-            }
-
-            download.result().ifPresentOrElse(
-                    result -> addLinkedFileFromURL(databaseContext, result, entry),
-                    () -> dialogService.notify(Localization.lang("No full text document found for entry %0.",
-                            entry.getCitationKey().orElse(Localization.lang("undefined")))));
+        BibEntry entry = download.entry();
+        if (!databaseContext.getDatabase().getEntries().contains(entry)) {
+            return;
         }
+        if (!entry.equals(download.lookupSnapshot())) {
+            LOGGER.debug("Entry changed during full text search; skipping download.");
+            return;
+        }
+
+        download.result().ifPresentOrElse(
+                result -> addLinkedFileFromURL(databaseContext, result, entry),
+                () -> dialogService.notify(Localization.lang("No full text document found for entry %0.",
+                        entry.getCitationKey().orElse(Localization.lang("undefined")))));
     }
 
     /// This method attaches a linked file from a URL (if not already linked) to an entry using the key and value pair

jabgui/src/test/java/org/jabref/gui/externalfiles/DownloadFullTextActionTest.java

@@ -6,7 +6,6 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
-import java.util.function.Consumer;
 import java.util.function.Function;
 
 import org.jabref.gui.DialogService;
@@ -42,9 +41,11 @@ class DownloadFullTextActionTest {
     private BibDatabaseContext databaseContext;
     private BibEntry entry;
     private FetcherResult fetcherResult;
+    private List<Runnable> pendingUiActions;
 
     @BeforeEach
     void setUp() throws MalformedURLException {
+        pendingUiActions = new ArrayList<>();
         dialogService = mock(DialogService.class);
         stateManager = new JabRefGuiStateManager();
         preferences = mock(GuiPreferences.class);
@@ -80,9 +81,9 @@ void skipsDownloadWhenEntryChangedAfterLookup() throws Exception {
         RecordingDownloadFullTextAction action = new RecordingDownloadFullTextAction(_ -> Optional.of(fetcherResult));
 
         BackgroundTask<?> task = captureTask(action);
-        Object downloads = task.call();
+        task.call();
         entry.withField(StandardField.TITLE, "Updated title");
-        runSuccessHandler(task, downloads);
+        runPendingUiActions();
 
         assertEquals(List.of(), action.downloadedEntries);
     }
@@ -92,9 +93,9 @@ void skipsDownloadWhenEntryDeletedAfterLookup() throws Exception {
         RecordingDownloadFullTextAction action = new RecordingDownloadFullTextAction(_ -> Optional.of(fetcherResult));
 
         BackgroundTask<?> task = captureTask(action);
-        Object downloads = task.call();
+        task.call();
         databaseContext.getDatabase().removeEntry(entry);
-        runSuccessHandler(task, downloads);
+        runPendingUiActions();
 
         assertEquals(List.of(), action.downloadedEntries);
     }
@@ -113,6 +114,31 @@ void finderReceivesEntrySnapshot() throws Exception {
         assertEquals(List.of(entry), action.downloadedEntries);
     }
 
+    @Test
+    void attachesEachEntryBeforeLookingUpTheNext() throws Exception {
+        BibEntry secondEntry = new BibEntry().withField(StandardField.DOI, "10.1000/second");
+        databaseContext.getDatabase().insertEntry(secondEntry);
+        stateManager.setSelectedEntries(List.of(entry, secondEntry));
+        List<BibEntry> attachedEntries = new ArrayList<>();
+        List<Integer> attachedBeforeLookup = new ArrayList<>();
+        DownloadFullTextAction action = new DownloadFullTextAction(dialogService, stateManager, preferences, taskExecutor, _ -> {
+            runPendingUiActions();
+            attachedBeforeLookup.add(attachedEntries.size());
+            return Optional.of(fetcherResult);
+        }, pendingUiActions::add) {
+            @Override
+            void addLinkedFileFromURL(BibDatabaseContext databaseContext, FetcherResult result, BibEntry entry) {
+                attachedEntries.add(entry);
+            }
+        };
+
+        BackgroundTask<?> task = captureTask(action);
+        completeTask(task);
+
+        assertEquals(List.of(0, 1), attachedBeforeLookup);
+        assertEquals(List.of(entry, secondEntry), attachedEntries);
+    }
+
     private BackgroundTask<?> captureTask(DownloadFullTextAction action) {
         action.execute();
 
@@ -121,25 +147,23 @@ private BackgroundTask<?> captureTask(DownloadFullTextAction action) {
         return taskCaptor.getValue();
     }
 
-    private static void completeTask(BackgroundTask<?> task) throws Exception {
-        Object downloads = task.call();
-        runSuccessHandler(task, downloads);
+    private void completeTask(BackgroundTask<?> task) throws Exception {
+        task.call();
+        runPendingUiActions();
     }
 
-    @SuppressWarnings({"rawtypes", "unchecked"})
-    private static void runSuccessHandler(BackgroundTask<?> task, Object downloads) {
-        Consumer onSuccess = task.getOnSuccess();
-        if (onSuccess != null) {
-            onSuccess.accept(downloads);
-        }
+    private void runPendingUiActions() {
+        List<Runnable> actions = List.copyOf(pendingUiActions);
+        pendingUiActions.clear();
+        actions.forEach(Runnable::run);
     }
 
     private class RecordingDownloadFullTextAction extends DownloadFullTextAction {
         private final List<BibEntry> downloadedEntries = new ArrayList<>();
         private final List<FetcherResult> downloadedResults = new ArrayList<>();
 
         RecordingDownloadFullTextAction(Function<BibEntry, Optional<FetcherResult>> fullTextFinder) {
-            super(dialogService, stateManager, preferences, taskExecutor, fullTextFinder);
+            super(dialogService, stateManager, preferences, taskExecutor, fullTextFinder, pendingUiActions::add);
         }
 
         @Override