Attach each full text document as soon as its search finishes

[koppor]

Summary

When full text documents are searched for several selected entries (Tools → Get fulltext), JabRef collected every result and attached them only after the last entry was looked up. With the browser-extension fulltext provider (JabRef#16124), one lookup can take up to minutes, so the browser visibly opened and closed tabs while nothing appeared in JabRef for a long time. Now each entry's document is attached as soon as its own search finishes. Cancelling the task keeps the documents already found instead of discarding all of them.

The existing safeguards still apply per entry: nothing is attached if the library was closed, or the entry was deleted or edited during its search.

Analogies

Before, results were like honey collected in a jar and handed over only when the jar was full; now each drop is served as soon as it drips. Like a box of chocolates, every entry is its own piece, and nobody has to wait for the whole box to be unwrapped. And just as the moon shows up in phases instead of all at once, attached files now appear one after another while the search runs.

jabref-contrib-policy:4.2:reviewed​:ok

Steps to test

1. Open a library and select about five entries that have a DOI of an open-access article (e.g., 10.1371/journal.pmed.0020124).
2. Choose Tools → Get fulltext and confirm the download.
3. Watch the file column while the background task shows "1/5 entries", "2/5 entries", ...: each entry gets its PDF linked as soon as the counter passes it, not only after "5/5 entries".

No screenshot: the change is in when files appear, which a still image cannot show.

Related issues and pull requests

Follow-up to JabRef#16831 (background task for the full text search). Noticed while testing JabRef#16124.

AI usage

Claude Code (model claude-opus-5[1m]), AIL3: the model diagnosed the cause and drafted the change and tests; the contributor reviews and owns them.

AI CHECKLIST.md walkthrough

1. Code self-review

Nullability and control flow

• No == null / != null checks — JSpecify annotations (@NullMarked, @Nullable, @NonNull) used instead.
• No Objects.requireNonNull(...) — nullability expressed via JSpecify annotations.
• [/] New classes annotated with @NullMarked (org.jspecify.annotations.NullMarked).
• [/] New packages annotated with @NullMarked (org.jspecify.annotations.NullMarked).
• Optional consumed with ifPresent / ifPresentOrElse / map / orElseThrow — never orElse(unusedValue) nor an isPresent() + get() block.
• [/] StringUtil.isBlank(...) used instead of s == null || s.isBlank().

Exceptions

• No catch (Exception e) — only specific exceptions are caught.
• No throw new RuntimeException(...) / IllegalStateException(...) — these tear down the whole application.
• [/] Logged exceptions are passed as the last logger argument (LOGGER.info("...", e)), not concatenated into the message string.

Style and idioms

• New BibEntry objects built with withers (withField, not setField).
• Modern Java used: List.of() / Map.of() / Set.of(), Path.of(), SequencedCollection / SequencedSet, text blocks.
• [/] Regexes use a precompiled Pattern.compile(...) constant, not String.matches(...).
• Keep alphabetical ordering when adding variables or enum items.
• Background work uses org.jabref.logic.util.BackgroundTask, not new Thread().
• No commented-out code, no trivial comments restating the code, no AI-disclosure comments in source.
• [/] Markdown Javadoc (///) uses Markdown syntax, not JavaDoc inline tags: `code` instead of {@code}, [ClassName] instead of {@link}.

User-facing text

• [/] All user-facing text localized (Localization.lang in Java, % prefix in FXML).
• [/] Sentence case (not Title Case); no trailing !; labels do not end with :.
• [/] Variance expressed with placeholders ("...: %0"), not string concatenation.

Security

• [/] User-controlled data (request params, entry fields, file contents) is HTML-escaped before being written into any text/html response — including exception/error messages, not just the success body (XSS).

Tests

• [/] Behavior changes in org.jabref.model / org.jabref.logic have added or updated tests.
• Tests assert object contents (assertEquals), use plain JUnit asserts (not AssertJ), have no @DisplayName, do not catch exceptions (let them propagate so JUnit reports setup/teardown failures directly), and use @TempDir instead of manual temp directories.
• [/] Fetcher tests hit the live endpoints — the remote API is not mocked or stubbed (automated-review suggestions to mock it are rejected on purpose).

2. Verification commands

• [/] ./gradlew :jablib:check (or ./gradlew check for all modules). Only jabgui changed; ran :jabgui:test --tests DownloadFullTextActionTest (5/5 green).
• ./gradlew checkstyleMain checkstyleTest checkstyleJmh.
• ./gradlew modernizer.
• ./gradlew --no-configuration-cache :rewriteDryRun reports no changes (run ./gradlew rewriteRun to fix).
• [/] ./gradlew javadoc.
• npx markdownlint-cli2 "docs/**/*.md" "*.md" (only if Markdown changed).
• npm ci && npm run textlint reports no misspellings (only if Markdown changed).
• [/] Only if formatting is still off after rewriteRun: docker run -v $(pwd):/github/workspace ghcr.io/leventebajczi/intellij-format:master "*.java" "" ".idea/codeStyles/Project.xml".

3. Documentation

• CHANGELOG.md entry added if the change is visible to the user (end-user wording, no extra blank lines, sorted in next to existing entries about the same component/feature). Link the issue if one exists; link the PR only when no issue exists. Use TODO as the placeholder when neither is known yet — never a fake number. No entry for fixes to changes that were themselves introduced after the last release (feature only in ## [Unreleased]) — update the existing unreleased entry instead if needed.
• Searched jabref/issues and jabref-koppor/issues for a related issue; linked only on a confident match, otherwise kept TODO (no closes/fixes for merely-similar issues).
• Requirement added to docs/requirements/<area>.md if the change is a new feature or significant bug fix (skip for refactors, minor fixes, and internal changes). Updated req~fetchers.fulltext-background-search~1.
• Developer documentation under docs/ updated if behavior or architecture changed.

4. Pull request

• PR body built from .github/PULL_REQUEST_TEMPLATE.md, every section filled.
• All checklist items kept and marked [x], [ ], or [/].
• [/] "Steps to test" is a numbered list with a cropped screenshot of the result for every visible change; no video (only allowed when another program is involved, e.g. drag and drop or push to an external application).
• All HTML comments removed from the PR body.
• PR created with gh pr create --body-file <file> (not --body).
• If CHANGELOG.md used a TODO placeholder (no issue confidently identified yet — an existing issue link always stays), the PR was opened as draft, the placeholder was replaced with the real PR-number link after PR creation, committed and pushed, and only then was the PR marked ready for review. If an issue is identified or created later, the link is switched to the issue.

Checklist

• I own the copyright of the code submitted and I license it under the MIT license
• If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
• I manually tested my changes in running JabRef (always required)
• I added JUnit tests for changes (if applicable)
• I added screenshots in the PR description (if change is visible to the user)
• I added one sentence (max 20 words) to CHANGELOG.md describing the change from the user's point of view (if the change is visible to the user)
• [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Attach full-text documents as each search completes

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Attaches each discovered full-text document immediately instead of batching multi-entry results.
• Preserves completed attachments when users cancel remaining full-text searches.
• Retains library, deletion, and entry-change safeguards for every result.

Diagram

sequenceDiagram
    actor User
    participant Action as Download action
    participant Task as Background task
    participant Finder as Full text finder
    participant UI as JavaFX thread
    participant Library as Library entry
    User->>Action: Start selected entries
    Action->>Task: Execute search
    loop Each entry until cancelled
        Task->>Finder: Search snapshot
        Finder-->>Task: Return result
        Task->>UI: Queue entry result
        UI->>Library: Validate library and entry
        alt Entry remains valid
            UI->>Library: Attach document
        else Closed, deleted, or changed
            UI-->>Action: Skip result
        end
    end
    User-->>Task: Cancel remaining searches

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. One background task per entry

• ➕ Allows independent entry lifecycle and potentially parallel lookups.
• ➕ Naturally exposes completion events for each entry.
• ➖ Introduces additional task-management and progress-aggregation complexity.
• ➖ Parallel requests could overload providers or produce unpredictable ordering.

2. Block until each UI update completes

• ➕ Guarantees attachment finishes before the next lookup starts.
• ➕ Makes ordering deterministic without queued callbacks.
• ➖ Blocks the background worker on the JavaFX thread.
• ➖ Increases deadlock and responsiveness risks during shutdown or heavy UI activity.

Recommendation: Keep the PR's single sequential background task with per-entry JavaFX callbacks. It delivers results promptly with minimal architectural change, preserves provider request ordering and existing validation safeguards, and avoids blocking either thread; the injected UI scheduler also makes timing behavior testable.

Files changed (4) +68 / -41

Bug fix (1) +27 / -25

DownloadFullTextAction.javaDispatch each full-text result immediately +27/-25

Dispatch each full-text result immediately

• Replaces end-of-task result batching with per-entry dispatch to the JavaFX thread. Existing checks for closed libraries and deleted or modified entries now run for each result before attachment, while cancellation leaves prior results intact.

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

Tests (1) +39 / -15

DownloadFullTextActionTest.javaVerify attachment occurs between sequential lookups +39/-15

Verify attachment occurs between sequential lookups

• Introduces a controllable UI-action queue and verifies the first entry is attached before the second lookup begins. Existing snapshot, mutation, and deletion tests now execute pending UI callbacks directly.

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

Documentation (2) +2 / -1

CHANGELOG.mdDocument prompt multi-entry full-text attachment +1/-0

Document prompt multi-entry full-text attachment

• Adds an Unreleased fix entry explaining that full-text documents no longer wait for every selected-entry search to finish.

CHANGELOG.md

fetchers.mdRequire per-entry full-text result application +1/-1

Require per-entry full-text result application

• Extends the background full-text search requirement to specify that each result is applied immediately after its lookup completes.

docs/requirements/fetchers.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗