ScienceDirect fulltext fetcher missing scidir-pdf link by faneeshh · Pull Request #15382 · JabRef/jabref

faneeshh · 2026-03-21T05:23:17Z

Related issues and pull requests

PR Description

The Elsevier API response for a ScienceDirect article can include a direct PDF link alongside the article landing page link (scidir) in its coredata.link array. The fetcher was only reading scidir and discarding scidir-pdf, so it always fell through to Jsoup scraping the article page which frequently fails. This fix reads scidir-pdf when present and returns it directly which skips the scrape entirely.

Steps to test

The scidir-pdf link only appears for institutional Elsevier API keys or certain open access articles so manual end to end testing requires an institutional key but the fix could easily be tested by the new unit test though which mocks the API response with a fixture containing all three link types and asserts the PDF URL is returned directly.

./gradlew :jablib:fetcherTest --tests "org.jabref.logic.importer.fetcher.ScienceDirectTest.findsPdfDirectlyWhenSciDirPdfPresent"

Checklist

I own the copyright of the code submitted and I license it under the MIT license
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 a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
I described the change in CHANGELOG.md in a way that can be understood by the average user (if 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

qodo-free-for-open-source-projects · 2026-03-21T05:23:39Z

Review Summary by Qodo

Support direct PDF links in ScienceDirect fetcher

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Added support for direct PDF links from Elsevier API responses
• Introduced DoiResolution record to track PDF availability
• Returns PDF directly when scidir-pdf link present, skipping scraping
• Added comprehensive unit test with mocked API response

Diagram

flowchart LR
  A["Elsevier API Response"] -->|Contains scidir-pdf link| B["DoiResolution with isPdfDirect=true"]
  A -->|Only scidir link| C["DoiResolution with isPdfDirect=false"]
  B -->|Return PDF directly| D["Skip Jsoup scraping"]
  C -->|Scrape article page| E["Extract PDF link via Jsoup"]

File Changes

1. jablib/src/main/java/org/jabref/logic/importer/fetcher/ScienceDirect.java 🐞 Bug fix +19/-7

Extract and prioritize scidir-pdf links from API

• Created DoiResolution record to encapsulate URL and PDF availability flag
• Modified getUrlByDoi() to extract and return both scidir and scidir-pdf links
• Added logic to return PDF directly when scidir-pdf link is present
• Updated findFullText() to check for direct PDF and skip Jsoup scraping when available
• Changed variable references from String urlFromDoi to resolved.url()

jablib/src/main/java/org/jabref/logic/importer/fetcher/ScienceDirect.java

2. jablib/src/test/java/org/jabref/logic/importer/fetcher/ScienceDirectTest.java 🧪 Tests +37/-1

Add test for direct PDF link handling

• Added imports for mocking HTTP requests and responses
• Added new test findsPdfDirectlyWhenSciDirPdfPresent() with mocked API response
• Test verifies PDF URL is returned directly when scidir-pdf link is present
• Fixed API key retrieval to use "Scopus" instead of ScienceDirect.FETCHER_NAME

jablib/src/test/java/org/jabref/logic/importer/fetcher/ScienceDirectTest.java

3. CHANGELOG.md 📝 Documentation +1/-0

Document ScienceDirect PDF fetcher fix
• Added entry documenting the fix for ScienceDirect PDF fetching issue #12161
CHANGELOG.md

qodo-free-for-open-source-projects · 2026-03-21T05:23:39Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. ~~Weak assertion on PDF URL~~ ☑ 📘 Rule violation ✓ Correctness

Description

The new unit test asserts only that the result URL contains("pdfft"), which can pass for incorrect
URLs and weakens regression protection. JabRef JUnit conventions prefer direct content assertions
over indirect assertTrue predicate checks.

Code

jablib/src/test/java/org/jabref/logic/importer/fetcher/ScienceDirectTest.java[R110-114]

+            Optional<URL> result = finder.findFullText(entry);
+
+            assertTrue(result.isPresent());
+            assertTrue(result.get().toString().contains("pdfft"));
+        }

Evidence
The checklist requires strong, content-based assertions and discourages indirect boolean checks; the
added test uses assertTrue with isPresent() and contains(...) instead of asserting the exact
expected Optional value.
AGENTS.md
AGENTS.md
jablib/src/test/java/org/jabref/logic/importer/fetcher/ScienceDirectTest.java[110-114]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test `findsPdfDirectlyWhenSciDirPdfPresent` uses weak predicate assertions (`isPresent()` and `contains(&amp;amp;quot;pdfft&amp;amp;quot;)`) instead of verifying the exact URL returned.
## Issue Context
This behavior change is intended to return the `scidir-pdf` link directly. The test should assert the full expected URL (or at minimum equality to the mocked `@href` value) to reliably prevent regressions.
## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/importer/fetcher/ScienceDirectTest.java[91-115]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Magic string "Scopus" in test~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

The modified test stubs getApiKey using the literal "Scopus" instead of reusing the existing
fetcher constant, increasing risk of typos and drift if the name changes. This reduces
maintainability and consistency across the codebase.

Code

jablib/src/test/java/org/jabref/logic/importer/fetcher/ScienceDirectTest.java[44]
+        when(importerPreferences.getApiKey("Scopus")).thenReturn(apiKey);

Evidence
The checklist requires maintainable code without avoidable duplication/magic values; the changed
line hardcodes the fetcher name where a shared constant exists.
AGENTS.md
jablib/src/test/java/org/jabref/logic/importer/fetcher/ScienceDirectTest.java[44-44]
jablib/src/main/java/org/jabref/logic/importer/fetcher/Scopus.java[32-33]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ScienceDirectTest` hardcodes the fetcher key name as `&amp;amp;quot;Scopus&amp;amp;quot;`, duplicating the value instead of using the existing shared constant.
## Issue Context
A constant already exists (`Scopus.FETCHER_NAME`). Using it reduces the risk of typos and keeps tests aligned if the fetcher name ever changes.
## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/importer/fetcher/ScienceDirectTest.java[42-46]
- jablib/src/main/java/org/jabref/logic/importer/fetcher/Scopus.java[32-33]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. DoiResolution name collision 🐞 Bug ⚙ Maintainability

Description

ScienceDirect introduces a nested record named DoiResolution, which shares the same simple name as
the existing public org.jabref.logic.importer.fetcher.DoiResolution class in the same package.
This name collision makes code navigation and future refactors more error-prone and confusing.

Code

jablib/src/main/java/org/jabref/logic/importer/fetcher/ScienceDirect.java[R137-140]

+    private record DoiResolution(String url, boolean isPdfDirect) { }
+
+    private DoiResolution getUrlByDoi(String doi) throws UnirestException {
   String sciLink = "";

Evidence
ScienceDirect now defines a private nested DoiResolution record, while the package already
contains a public DoiResolution fetcher class; having two different types with the same simple
name in the same package increases the chance of confusion during maintenance and review.
jablib/src/main/java/org/jabref/logic/importer/fetcher/ScienceDirect.java[137-140]
jablib/src/main/java/org/jabref/logic/importer/fetcher/DoiResolution.java[31-36]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ScienceDirect` introduces a nested record named `DoiResolution`, but the same package already has a public class named `DoiResolution`. This simple-name collision is confusing and makes future refactors/imports/navigation riskier than necessary.
### Issue Context
This is purely a naming/maintainability problem (no runtime failure), but it violates the “least surprise” principle because developers already associate `DoiResolution` with the existing DOI-redirect fulltext fetcher.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/importer/fetcher/ScienceDirect.java[55-66]
- jablib/src/main/java/org/jabref/logic/importer/fetcher/ScienceDirect.java[137-170] շրջ
### Suggested change
Rename the nested record to something unique and specific (e.g., `ScienceDirectLinkResolution`, `ElsevierDoiResolution`, or `SciDirResolution`) and update references (`resolved` type + return type of `getUrlByDoi`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

calixtus · 2026-03-21T07:44:13Z

    }

-    private String getUrlByDoi(String doi) throws UnirestException {
+    private record DoiResolution(String url, boolean isPdfDirect) { }


I really dont like this tuple-results. Needs special knowledge to be used on the callers side. A blank String could also be misinterpreted.
How about a sealed interface with encoded types? Modern Java allows data-oriented patterns.

sealed interface DoiResolution permits Pdf, ScienceDirect, NotFound {} record Pdf(String url) implements DoiResolution {} record ScienceDirect(String url) implements DoiResolution {} record NotFound() implements DoiResolution {}

Hmm fair enough. Should be fine now.

…ix-12161

testlens-app · 2026-03-24T13:05:44Z

✅ All tests passed ✅

🏷️ Commit: 097e9bf
▶️ Tests: 10203 executed
⚪️ Checks: 52/52 completed

Learn more about TestLens at testlens.app.

* Use scidir-pdf link from Elsevier API response when available * Changelog * use sealed interface for DoiResolution * Refactor for readability * Fix formatting --------- Co-authored-by: Carl Christian Snethlage <calixtus@users.noreply.github.com>

faneeshh added 2 commits March 20, 2026 14:44

Use scidir-pdf link from Elsevier API response when available

3437a98

Changelog

d6fa163

github-actions Bot added the status: changes-required Pull requests that are not yet complete label Mar 21, 2026

github-actions Bot added good second issue Issues that involve a tour of two or three interweaved components in JabRef component: fetcher labels Mar 21, 2026

Merge branch 'main' into fix-12161

7c7ebe9

qodo-free-for-open-source-projects Bot reviewed Mar 21, 2026

View reviewed changes

Comment thread jablib/src/test/java/org/jabref/logic/importer/fetcher/ScienceDirectTest.java

This comment has been minimized.

Sign in to view

github-actions Bot mentioned this pull request Mar 21, 2026

Obtain fulltext from sciencedirect #12161

Closed

2 tasks

calixtus reviewed Mar 21, 2026

View reviewed changes

faneeshh added 2 commits March 21, 2026 16:23

use sealed interface for DoiResolution

6e731b0

Merge branch 'fix-12161' of https://github.com/faneeshh/jabref into f…

460d226

…ix-12161

This comment has been minimized.

Sign in to view

Merge branch 'main' into fix-12161

8c23137

This comment has been minimized.

Sign in to view

github-actions Bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Mar 21, 2026

Refactor for readability

e659621

This comment has been minimized.

Sign in to view

Fix formatting

097e9bf

calixtus approved these changes Mar 24, 2026

View reviewed changes

calixtus added this pull request to the merge queue Mar 24, 2026

github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Mar 24, 2026

Merged via the queue into JabRef:main with commit 2752d33 Mar 24, 2026
52 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ScienceDirect fulltext fetcher missing scidir-pdf link#15382

ScienceDirect fulltext fetcher missing scidir-pdf link#15382
calixtus merged 8 commits into
JabRef:mainfrom
faneeshh:fix-12161

faneeshh commented Mar 21, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Mar 21, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Mar 21, 2026 •

edited

Loading

Uh oh!

Uh oh!

This comment has been minimized.

calixtus Mar 21, 2026 •

edited

Loading

Uh oh!

faneeshh Mar 22, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

testlens-app Bot commented Mar 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

faneeshh commented Mar 21, 2026

Related issues and pull requests

PR Description

Steps to test

Checklist

Uh oh!

qodo-free-for-open-source-projects Bot commented Mar 21, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects Bot commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

This comment has been minimized.

calixtus Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

faneeshh Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

testlens-app Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All tests passed ✅

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qodo-free-for-open-source-projects Bot commented Mar 21, 2026 •

edited

Loading

calixtus Mar 21, 2026 •

edited

Loading

testlens-app Bot commented Mar 24, 2026 •

edited

Loading