Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shortDOI formatter is very slow #10589

Closed
stertooy opened this issue Oct 26, 2023 · 3 comments · Fixed by #10612
Closed

shortDOI formatter is very slow #10589

stertooy opened this issue Oct 26, 2023 · 3 comments · Fixed by #10612
Assignees
Labels
bug Confirmed bugs or reports that are very likely to be bugs good first issue An issue intended for project-newcomers. Varies in difficulty. type: performance

Comments

@stertooy
Copy link

In one of my libraries, which contains approx 100 DOI entries, having the shortDOI field formatter active noticeably slows down the time it takes to save the library.
It takes <1 second to save without said formatter, but 30-40 seconds with, during which JabRef becomes unresponsive. This happens on every save, even when all DOI fields are already in the shortDOI format.

@Override
public String format(String value) {
Objects.requireNonNull(value);
return DOI.parse(value)
.map(doi -> {
try {
return new ShortDOIService().getShortDOI(doi).getDOI();
} catch (ShortDOIServiceException e) {
LOGGER.error(e.getMessage(), e);
return value;
}
}).orElse(value);
}

public DOI getShortDOI(DOI doi) throws ShortDOIServiceException {
JSONObject responseJSON = makeRequest(doi);
String shortDoi = responseJSON.getString("ShortDOI");
return new DOI(shortDoi);
}

If I interpret the above pieces of code correctly, the formatter always makes a request to the shortDOI service, even if the DOI is already shortened. Would it be possible to first check if the DOI is already shortened (i.e. starts with "10/"), and only make a request if it is not?

@Siedlerchr
Copy link
Member

Thanks for the report and your analysis. Sounds like a good idea.

Solution: check if the DOI starts with the Pattern 10/[a-z]+ should be sufficient.

For future:
Ideally, the request should be executed in a background thread as well, but this would be a bit more complex from the current architecture and would require some major refactoring.

@github-project-automation github-project-automation bot moved this to Free to take in Good First Issues Oct 26, 2023
@Siedlerchr Siedlerchr added the good first issue An issue intended for project-newcomers. Varies in difficulty. label Oct 26, 2023
@github-project-automation github-project-automation bot moved this to Normal priority in Prioritization Oct 26, 2023
@koppor koppor added the bug Confirmed bugs or reports that are very likely to be bugs label Oct 28, 2023
@michalfarago
Copy link
Contributor

michalfarago commented Oct 30, 2023

Hello, could I work on this? Thank you.

@Siedlerchr Siedlerchr moved this from Free to take to Reserved in Good First Issues Oct 30, 2023
@stertooy
Copy link
Author

Solution: check if the DOI starts with the Pattern 10/[a-z]+ should be sufficient.

Although the shortDOI website says that shortDOI's are "of the form 10/abcde", the "abcde" part often contains numbers as well (e.g. 10/grv3jv). Also taking into account that DOI's are case insensitive, perhaps 10/[a-zA-Z0-9]+ would be better?

@github-project-automation github-project-automation bot moved this from Normal priority to Done in Prioritization Nov 2, 2023
@github-project-automation github-project-automation bot moved this from Reserved to Done in Good First Issues Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs good first issue An issue intended for project-newcomers. Varies in difficulty. type: performance
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants