Skip to content

Conversation

@koppor
Copy link
Member

@koppor koppor commented Nov 21, 2025

Fixes #14089

Was a bit larger than I thought; the DoiFetcher code needed a cleanup. There was also a space inserted. I don't know why this disappeared when reading from the String; maybe there was some automatic trimming. I added DoiFetcher.http for easy manual checking of the returned file.

Steps to test

Just see the fetcher tests passing.

Mandatory checks

  • 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 described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
  • [/] I checked the user documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

@koppor koppor added dev: code-quality Issues related to code or architecture decisions component: fetcher labels Nov 21, 2025
@koppor koppor changed the title Use steraming when downloading from DOI Use streaming when downloading from DOI Nov 21, 2025
@koppor
Copy link
Member Author

koppor commented Nov 21, 2025

Format fails because formatter does not work with Markdown properly (yet). EAP does.

--- a/jablib/src/main/java/org/jabref/logic/net/URLDownload.java
+++ b/jablib/src/main/java/org/jabref/logic/net/URLDownload.java
@@ -61,7 +61,7 @@ import org.slf4j.LoggerFactory;
 /// String content = dl.asString(ENCODING);
 /// dl.toFile(Path);              // available in FILE
 /// String contentType = dl.getMimeType();
-/// ```
+///```
 ///
 /// Almost every call to a public method creates a new HTTP connection
 /// (except for {@link #asString(Charset, URLConnection) asString},

I filed https://github.com/leventeBajczi/intellij-idea-format/pull/4/files to check whether the latest version works.

}

private void setPageCountToArticleId(BibEntry entry, String doiAsString) {
private void setPageNumbersBasedOnDoi(BibEntry entry, DOI doi) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Count and page numbers are two different thing
E.g total pages

Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests


/// Trims whitespaces at the beginning of the BibEntry
public void trimLeft() {
this.parsedSerialization = parsedSerialization.trim(); // we should do "trimLeft", but currently, it is OK as is.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test

@github-actions github-actions bot added the status: changes-required Pull requests that are not yet complete label Nov 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: fetcher dev: code-quality Issues related to code or architecture decisions status: changes-required Pull requests that are not yet complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

URLDownload should include httpResponse in exception

3 participants