diff --git a/CHANGELOG.md b/CHANGELOG.md index d380fd1bfb60..00870b063457 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - 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 pressing "+" in the "File" field with an automatically found file selected opened the "Add file link" dialog instead of linking the selected file. [#16938](https://github.com/JabRef/jabref/pull/16938) +- We fixed an issue where failed full text downloads opened an error dialog for each entry. [#774](https://github.com/JabRef/jabref-koppor/pull/774) - We fixed an issue where case-sensitive search (`=!`, `==!`, `=~!`) in linked files ignored the casing and matched text in any casing. [#13048](https://github.com/JabRef/jabref/issues/13048) ### Removed diff --git a/jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java b/jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java index 4fd5f1b4eec3..36fed0d90c7a 100644 --- a/jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java +++ b/jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java @@ -18,6 +18,7 @@ import javafx.beans.property.SimpleDoubleProperty; import org.jabref.gui.DialogService; +import org.jabref.gui.Notifications; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.externalfiletype.ExternalFileType; import org.jabref.gui.externalfiletype.ExternalFileTypes; @@ -222,15 +223,19 @@ private void onSuccess(Path targetDirectory, Path downloadedFile) { } } + /// Notifies instead of opening a modal dialog: the download runs in the background, often as one of many + /// (bulk full text download, import), and a blocked publisher must not stop the user with a dialog per entry. private void onFailure(URLDownload urlDownload, Exception ex) { LOGGER.error("Error downloading from URL: {}", urlDownload, ex); - if (ex instanceof FetcherException fetcherException) { - dialogService.showErrorDialogAndWait(fetcherException); - } else { - String fetcherExceptionMessage = ex.getLocalizedMessage(); - String failedTitle = Localization.lang("Failed to download from URL"); - dialogService.showErrorDialogAndWait(failedTitle, Localization.lang("Please check the URL and try again.\nURL: %0\nDetails: %1", urlDownload.getSource(), fetcherExceptionMessage)); - } + // The response body (often a whole HTML page) stays in the log + String reason = Optional.of(ex) + .filter(FetcherException.class::isInstance) + .flatMap(e -> ((FetcherException) e).getHttpResponse()) + .map(response -> "HTTP %d %s".formatted(response.statusCode(), response.responseMessage())) + .orElseGet(ex::getLocalizedMessage); + dialogService.notify(new Notifications.UndefinedNotification( + Localization.lang("Failed to download from URL"), + "%s\n%s\n%s".formatted(entry.getCitationKey().orElse(""), FetcherException.getRedactedUrl(urlDownload.getSource().toString()), reason).strip())); } private boolean checkSSLHandshake(URLDownload urlDownload) { diff --git a/jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java b/jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java index d77897c205ab..2ee669f675b5 100644 --- a/jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java +++ b/jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java @@ -19,6 +19,7 @@ import javafx.scene.control.DialogPane; import org.jabref.gui.DialogService; +import org.jabref.gui.Notifications; import org.jabref.gui.externalfiletype.ExternalFileType; import org.jabref.gui.externalfiletype.ExternalFileTypes; import org.jabref.gui.externalfiletype.StandardExternalFileType; @@ -45,6 +46,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -57,6 +59,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; // Need to run on JavaFX thread since {@link org.jabref.gui.linkedfile.DeleteFileAction.execute} creates a DialogPane @@ -112,13 +115,17 @@ void tearDown() { /// Serves the given bytes for every request on a random free port, so download tests do not depend on external sites private String serve(String contentType, byte[] body) throws IOException { + return serve(200, contentType, body); + } + + private String serve(int status, String contentType, byte[] body) throws IOException { httpServer = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); httpServer.createContext("/", exchange -> { exchange.getResponseHeaders().add("Content-Type", contentType); if ("HEAD".equals(exchange.getRequestMethod())) { exchange.sendResponseHeaders(200, -1); } else { - exchange.sendResponseHeaders(200, body.length); + exchange.sendResponseHeaders(status, body.length); exchange.getResponseBody().write(body); } exchange.close(); @@ -234,6 +241,25 @@ void downloadHtmlFileCausesWarningDisplay(Boolean keepHtmlLink, String warningSu verify(dialogService, atLeastOnce()).notify(warningText); } + @Test + void failedDownloadNotifiesInsteadOfOpeningDialog() throws IOException { + when(filePreferences.shouldStoreFilesRelativeToBibFile()).thenReturn(true); + when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]"); + when(filePreferences.getFileDirectoryPattern()).thenReturn(""); + databaseContext.setDatabasePath(tempFile); + String serverUrl = serve(403, "text/html", "Just a moment...".getBytes(StandardCharsets.UTF_8)); + linkedFile = new LinkedFile(URLUtil.create(serverUrl), ""); + + LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, preferences); + viewModel.download(false, new JabRefUndoManager()); + + ArgumentCaptor notification = ArgumentCaptor.forClass(Notifications.UndefinedNotification.class); + verify(dialogService).notify(notification.capture()); + verifyNoMoreInteractions(dialogService); + assertEquals("Failed to download from URL", notification.getValue().getTitle()); + assertEquals("asdf\n" + serverUrl + "\nHTTP 403 Forbidden", notification.getValue().getSummary()); + } + @Test void isNotSamePath() { linkedFile = new LinkedFile("desc", tempFile, "pdf"); diff --git a/jablib/src/main/resources/l10n/JabRef_en.properties b/jablib/src/main/resources/l10n/JabRef_en.properties index 63c933b56930..4e7b323d7c9a 100644 --- a/jablib/src/main/resources/l10n/JabRef_en.properties +++ b/jablib/src/main/resources/l10n/JabRef_en.properties @@ -3001,7 +3001,6 @@ Access\ denied.\ You\ do\ not\ have\ permission\ to\ access\ this\ resource.\ Pl The\ requested\ resource\ could\ not\ be\ found.\ It\ seems\ that\ the\ file\ you\ are\ trying\ to\ download\ is\ not\ available\ or\ has\ been\ moved.\ Please\ verify\ the\ URL\ and\ try\ again.\ If\ you\ believe\ this\ is\ an\ error,\ please\ contact\ the\ administrator\ for\ further\ assistance.=The requested resource could not be found. It seems that the file you are trying to download is not available or has been moved. Please verify the URL and try again. If you believe this is an error, please contact the administrator for further assistance. Something\ is\ wrong\ on\ JabRef\ side.\ Please\ check\ the\ URL\ and\ try\ again.=Something is wrong on JabRef side. Please check the URL and try again. Error\ downloading\ from\ URL.\ Cause\ is\ likely\ the\ server\ side.\nPlease\ try\ again\ later\ or\ contact\ the\ server\ administrator.=Error downloading from URL. Cause is likely the server side.\nPlease try again later or contact the server administrator. -Please\ check\ the\ URL\ and\ try\ again.\nURL\:\ %0\nDetails\:\ %1=Please check the URL and try again.\nURL: %0\nDetails: %1 Finished=Finished Finished\ writing\ metadata\ for\ library\ %0\ (%1\ succeeded,\ %2\ skipped,\ %3\ errors).=Finished writing metadata for library %0 (%1 succeeded, %2 skipped, %3 errors).