Remove download when html file

CHANGELOG.md

@@ -23,14 +23,15 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 ### Changed
 
 - We replaced the word "Key bindings" with "Keyboard shortcuts" in the Preferences tab. [#11153](https://github.com/JabRef/jabref/pull/11153)
-- We slightly improved the duplicate check if ISBN numbers are present. [#8885](https://github.com/JabRef/jabref/issues/8885)
+- We slightly improved the duplicate check if ISBNs are present. [#8885](https://github.com/JabRef/jabref/issues/8885)
+- JabRef no longer downloads HTML files of websites when a PDF was not found. [#10149](https://github.com/JabRef/jabref/issues/10149)
 
 ### Fixed
 
-- We fixed an issue where entry type with duplicate fields prevented opening existing libraries with custom entry types [#11127](https://github.com/JabRef/jabref/issues/11127)
+- We fixed an issue where entry type with duplicate fields prevented opening existing libraries with custom entry types. [#11127](https://github.com/JabRef/jabref/issues/11127)
 - We fixed an issue where Markdown rendering removed braces from the text. [#10928](https://github.com/JabRef/jabref/issues/10928)
 - We fixed an issue when the file was flagged as changed on disk in the case of content selectors or groups. [#9064](https://github.com/JabRef/jabref/issues/9064)
-- We fixed crash on opening the entry editor when auto completion is enabled. [#11188](https://github.com/JabRef/jabref/issues/11188)
+- We fixed crash on opening the entry editor when auto-completion is enabled. [#11188](https://github.com/JabRef/jabref/issues/11188)
 - We fixed the usage of the key binding for "Clear search" (default: <kbd>Escape</kbd>). [#10764](https://github.com/JabRef/jabref/issues/10764)
 - We fixed an issue where library shown as unsaved and marked (*) after accepting changes made externally to the file. [#11027](https://github.com/JabRef/jabref/issues/11027)
 

src/main/java/org/jabref/gui/entryeditor/EntryEditor.java

@@ -74,6 +74,8 @@
  * <p>
  * EntryEditor also registers itself to the event bus, receiving events whenever a field of the entry changes, enabling
  * the text fields to update themselves if the change is made from somewhere else.
+ * <p>
+ * The editors for fields are created via {@link org.jabref.gui.fieldeditors.FieldEditors}.
  */
 public class EntryEditor extends BorderPane {
 

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

@@ -141,8 +141,7 @@ private void addLinkedFileFromURL(BibDatabaseContext databaseContext, URL url, B
                     taskExecutor,
                     dialogService,
                     preferences);
-
-            onlineFile.download();
+            onlineFile.download(true);
         } else {
             dialogService.notify(Localization.lang("Full text document for entry %0 already linked.",
                     entry.getCitationKey().orElse(Localization.lang("undefined"))));

src/main/java/org/jabref/gui/externalfiles/ImportHandler.java

@@ -276,7 +276,7 @@ public void downloadLinkedFiles(BibEntry entry) {
                                  taskExecutor,
                                  dialogService,
                                  preferencesService
-                         ).download()
+                         ).download(false)
                  );
         }
     }
@@ -405,13 +405,13 @@ public void importEntriesWithDuplicateCheck(BibDatabaseContext database, List<Bi
         boolean firstEntry = true;
         for (BibEntry entry : entriesToAdd) {
             if (firstEntry) {
-                LOGGER.debug("First entry to import, we use BREAK");
+                LOGGER.debug("First entry to import, we use BREAK (\"Ask every time\") as decision");
                 importEntryWithDuplicateCheck(database, entry, BREAK);
                 firstEntry = false;
                 continue;
             }
             if (preferencesService.getMergeDialogPreferences().shouldMergeApplyToAllEntries()) {
-                var decision = preferencesService.getMergeDialogPreferences().getAllEntriesDuplicateResolverDecision();
+                DuplicateResolverDialog.DuplicateResolverResult decision = preferencesService.getMergeDialogPreferences().getAllEntriesDuplicateResolverDecision();
                 LOGGER.debug("Not first entry, pref flag is true, we use {}", decision);
                 importEntryWithDuplicateCheck(database, entry, decision);
             } else {

src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java

@@ -384,8 +384,11 @@ public void edit() {
         });
     }
 
+    /**
+     * @implNote Similar method {@link org.jabref.gui.linkedfile.RedownloadMissingFilesAction#redownloadMissing}
+     */
     public void redownload() {
-        LOGGER.info("Redownloading file from " + linkedFile.getSourceUrl());
+        LOGGER.info("Redownloading file from {}", linkedFile.getSourceUrl());
         if (linkedFile.getSourceUrl().isEmpty() || !LinkedFile.isOnlineLink(linkedFile.getSourceUrl())) {
             throw new UnsupportedOperationException("In order to download the file, the source url has to be an online link");
         }
@@ -400,13 +403,14 @@ public void redownload() {
                 dialogService,
                 preferencesService.getFilePreferences(),
                 taskExecutor,
-                fileName);
+                fileName,
+                true);
         downloadProgress.bind(downloadLinkedFileAction.downloadProgress());
         downloadLinkedFileAction.execute();
     }
 
-    public void download() {
-        LOGGER.info("Downloading file from " + linkedFile.getSourceUrl());
+    public void download(boolean keepHtmlLink) {
+        LOGGER.info("Downloading file from {}", linkedFile.getSourceUrl());
         if (!linkedFile.isOnlineLink()) {
             throw new UnsupportedOperationException("In order to download the file it has to be an online link");
         }
@@ -418,7 +422,9 @@ public void download() {
                 linkedFile.getLink(),
                 dialogService,
                 preferencesService.getFilePreferences(),
-                taskExecutor);
+                taskExecutor,
+                "",
+                keepHtmlLink);
         downloadProgress.bind(downloadLinkedFileAction.downloadProgress());
         downloadLinkedFileAction.execute();
     }

src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java

@@ -376,7 +376,7 @@ public void execute() {
                 case EDIT_FILE_LINK -> linkedFile.edit();
                 case OPEN_FILE -> linkedFile.open();
                 case OPEN_FOLDER -> linkedFile.openFolder();
-                case DOWNLOAD_FILE -> linkedFile.download();
+                case DOWNLOAD_FILE -> linkedFile.download(true);
                 case REDOWNLOAD_FILE -> linkedFile.redownload();
                 case RENAME_FILE_TO_PATTERN -> linkedFile.renameToSuggestion();
                 case RENAME_FILE_TO_NAME -> linkedFile.askForNameAndRename();

src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java

@@ -279,7 +279,7 @@ private void addFromURLAndDownload(URL url) {
                 dialogService,
                 preferences);
         files.add(onlineFile);
-        onlineFile.download();
+        onlineFile.download(true);
     }
 
     public void deleteFile(LinkedFileViewModel file) {

src/main/java/org/jabref/gui/linkedfile/AttachFileFromURLAction.java

@@ -68,7 +68,7 @@ public void execute() {
                              taskExecutor,
                              dialogService,
                              preferencesService);
-            onlineFile.download();
+            onlineFile.download(true);
         } catch (MalformedURLException exception) {
             dialogService.showErrorDialogAndWait(Localization.lang("Invalid URL"), exception);
         }

src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java

@@ -6,6 +6,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Optional;
 
 import javax.net.ssl.HostnameVerifier;
@@ -55,6 +57,7 @@ public class DownloadLinkedFileAction extends SimpleCommand {
     private final String downloadUrl;
     private final FilePreferences filePreferences;
     private final TaskExecutor taskExecutor;
+    private final boolean keepHtmlLink;
 
     private final BibDatabaseContext databaseContext;
 
@@ -68,7 +71,8 @@ public DownloadLinkedFileAction(BibDatabaseContext databaseContext,
                                     DialogService dialogService,
                                     FilePreferences filePreferences,
                                     TaskExecutor taskExecutor,
-                                    String suggestedName) {
+                                    String suggestedName,
+                                    boolean keepHtmlLink) {
         this.databaseContext = databaseContext;
         this.entry = entry;
         this.linkedFile = linkedFile;
@@ -77,23 +81,27 @@ public DownloadLinkedFileAction(BibDatabaseContext databaseContext,
         this.dialogService = dialogService;
         this.filePreferences = filePreferences;
         this.taskExecutor = taskExecutor;
+        this.keepHtmlLink = keepHtmlLink;
 
         this.linkedFileHandler = new LinkedFileHandler(linkedFile, entry, databaseContext, filePreferences);
     }
 
+    /**
+     * Downloads the given linked file to the first existing file directory. It keeps HTML files as URLs.
+     */
     public DownloadLinkedFileAction(BibDatabaseContext databaseContext,
                                     BibEntry entry,
                                     LinkedFile linkedFile,
                                     String downloadUrl,
                                     DialogService dialogService,
                                     FilePreferences filePreferences,
                                     TaskExecutor taskExecutor) {
-        this(databaseContext, entry, linkedFile, downloadUrl, dialogService, filePreferences, taskExecutor, "");
+        this(databaseContext, entry, linkedFile, downloadUrl, dialogService, filePreferences, taskExecutor, "", true);
     }
 
     @Override
     public void execute() {
-        LOGGER.info("Downloading file from " + downloadUrl);
+        LOGGER.info("Downloading file from {}", downloadUrl);
         if (downloadUrl.isEmpty() || !LinkedFile.isOnlineLink(downloadUrl)) {
             throw new UnsupportedOperationException("In order to download the file, the url has to be an online link");
         }
@@ -132,51 +140,64 @@ private void doDownload(Path targetDirectory, URLDownload urlDownload) {
         taskExecutor.execute(downloadTask);
     }
 
-    private void onSuccess(Path targetDirectory, Path destination) {
+    /**
+     * @param targetDirectory The directory to store the file into. Is an absolute path.
+     */
+    private void onSuccess(Path targetDirectory, Path downloadedFile) {
+        assert targetDirectory.isAbsolute();
+
         boolean isDuplicate;
         boolean isHtml;
         try {
-            isDuplicate = FileNameUniqueness.isDuplicatedFile(targetDirectory, destination.getFileName(), dialogService);
+            isDuplicate = FileNameUniqueness.isDuplicatedFile(targetDirectory, downloadedFile.getFileName(), dialogService);
         } catch (IOException e) {
             LOGGER.error("FileNameUniqueness.isDuplicatedFile failed", e);
             return;
         }
 
         if (isDuplicate) {
-            destination = targetDirectory.resolve(
-                    FileNameUniqueness.eraseDuplicateMarks(destination.getFileName()));
-
-            linkedFile.setLink(FileUtil.relativize(destination,
-                    databaseContext.getFileDirectories(filePreferences)).toString());
+            // We do not add duplicate files.
+            // The downloaded file was deleted in {@link org.jabref.logic.util.io.FileNameUniqueness.isDuplicatedFile]}
+            LOGGER.info("File {} already exists in target directory {}.", downloadedFile.getFileName(), targetDirectory);
+            return;
+        }
 
-            isHtml = linkedFile.getFileType().equals(StandardExternalFileType.URL.getName());
+        // we need to call LinkedFileViewModel#fromFile, because we need to make the path relative to the configured directories
+        LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(
+                downloadedFile,
+                databaseContext.getFileDirectories(filePreferences),
+                filePreferences);
+        if (newLinkedFile.getDescription().isEmpty() && !linkedFile.getDescription().isEmpty()) {
+            newLinkedFile.setDescription((linkedFile.getDescription()));
+        }
+        if (linkedFile.getSourceUrl().isEmpty() && LinkedFile.isOnlineLink(linkedFile.getLink())) {
+            newLinkedFile.setSourceURL(linkedFile.getLink());
         } else {
-            // we need to call LinkedFileViewModel#fromFile, because we need to make the path relative to the configured directories
-            LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(
-                    destination,
-                    databaseContext.getFileDirectories(filePreferences),
-                    filePreferences);
-            if (newLinkedFile.getDescription().isEmpty() && !linkedFile.getDescription().isEmpty()) {
-                newLinkedFile.setDescription((linkedFile.getDescription()));
-            }
-            if (linkedFile.getSourceUrl().isEmpty() && LinkedFile.isOnlineLink(linkedFile.getLink())) {
-                newLinkedFile.setSourceURL(linkedFile.getLink());
-            } else {
-                newLinkedFile.setSourceURL(linkedFile.getSourceUrl());
-            }
-            entry.replaceDownloadedFile(linkedFile.getLink(), newLinkedFile);
-            isHtml = newLinkedFile.getFileType().equals(StandardExternalFileType.URL.getName());
+            newLinkedFile.setSourceURL(linkedFile.getSourceUrl());
         }
 
-        // Notify in bar when the file type is HTML.
+        isHtml = newLinkedFile.getFileType().equals(StandardExternalFileType.URL.getName());
         if (isHtml) {
-            dialogService.notify(Localization.lang("Downloaded website as an HTML file."));
-            LOGGER.debug("Downloaded website {} as an HTML file at {}", linkedFile.getLink(), destination);
+            if (this.keepHtmlLink) {
+                dialogService.notify(Localization.lang("Download '%0' was a HTML file. Keeping URL.", downloadUrl));
+            } else {
+                dialogService.notify(Localization.lang("Download '%0' was a HTML file. Removed.", downloadUrl));
+                List<LinkedFile> newFiles = new ArrayList<>(entry.getFiles());
+                newFiles.remove(linkedFile);
+                entry.setFiles(newFiles);
+                try {
+                    Files.delete(downloadedFile);
+                } catch (IOException e) {
+                    LOGGER.error("Could not delete downloaded file {}.", downloadedFile, e);
+                }
+            }
+        } else {
+            entry.replaceDownloadedFile(linkedFile.getLink(), newLinkedFile);
         }
     }
 
     private void onFailure(URLDownload urlDownload, Exception ex) {
-        LOGGER.error("Error downloading from URL: " + urlDownload, ex);
+        LOGGER.error("Error downloading from URL: {}", urlDownload, ex);
         String fetcherExceptionMessage = ex.getMessage();
         String failedTitle = Localization.lang("Failed to download from URL");
         int statusCode;
@@ -261,7 +282,7 @@ private Optional<ExternalFileType> inferFileTypeFromMimeType(URLDownload urlDown
         String mimeType = urlDownload.getMimeType();
 
         if (mimeType != null) {
-            LOGGER.debug("MIME Type suggested: " + mimeType);
+            LOGGER.debug("MIME Type suggested: {}", mimeType);
             return ExternalFileTypes.getExternalFileTypeByMimeType(mimeType, filePreferences);
         } else {
             return Optional.empty();

src/main/java/org/jabref/gui/linkedfile/RedownloadMissingFilesAction.java

@@ -56,6 +56,9 @@ public void execute() {
         }
     }
 
+    /**
+     * @implNote Similar method {@link org.jabref.gui.fieldeditors.LinkedFileViewModel#redownload}
+     */
     private void redownloadMissing(BibDatabaseContext databaseContext) {
         LOGGER.info("Redownloading missing files");
         databaseContext.getEntries().forEach(entry -> {
@@ -71,7 +74,7 @@ private void redownloadMissing(BibDatabaseContext databaseContext) {
                 String fileName = Path.of(linkedFile.getLink()).getFileName().toString();
 
                 DownloadLinkedFileAction downloadAction = new DownloadLinkedFileAction(this.databaseContext, entry,
-                        linkedFile, linkedFile.getSourceUrl(), dialogService, filePreferences, taskExecutor, fileName);
+                        linkedFile, linkedFile.getSourceUrl(), dialogService, filePreferences, taskExecutor, fileName, true);
                 downloadAction.execute();
             });
         });

src/main/java/org/jabref/logic/util/io/FileNameUniqueness.java

@@ -11,7 +11,12 @@
 import org.jabref.gui.DialogService;
 import org.jabref.logic.l10n.Localization;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public class FileNameUniqueness {
+    private static final Logger LOGGER = LoggerFactory.getLogger(FileNameUniqueness.class);
+
     private static final Pattern DUPLICATE_MARK_PATTERN = Pattern.compile("(.*) \\(\\d+\\)");
 
     /**
@@ -61,7 +66,6 @@ public static String getNonOverWritingFileName(Path targetDirectory, String file
      * @throws IOException Fail when the file is not exist or something wrong when reading the file
      */
     public static boolean isDuplicatedFile(Path directory, Path fileName, DialogService dialogService) throws IOException {
-
         Objects.requireNonNull(directory);
         Objects.requireNonNull(fileName);
         Objects.requireNonNull(dialogService);
@@ -83,10 +87,11 @@ public static boolean isDuplicatedFile(Path directory, Path fileName, DialogServ
 
         while (Files.exists(originalFile)) {
             if (com.google.common.io.Files.equal(originalFile.toFile(), duplicateFile.toFile())) {
-                if (duplicateFile.toFile().delete()) {
+                try {
+                    Files.delete(duplicateFile);
                     dialogService.notify(Localization.lang("File '%1' is a duplicate of '%0'. Keeping '%0'", originalFileName, fileName));
-                } else {
-                    dialogService.notify(Localization.lang("File '%1' is a duplicate of '%0'. Keeping both due to deletion error", originalFileName, fileName));
+                } catch (IOException e) {
+                    LOGGER.error("File '{}' is a duplicate of '{}'. Could not delete '{}'.", fileName, originalFileName, fileName);
                 }
                 return true;
             }

src/main/java/org/jabref/model/entry/BibEntry.java

@@ -613,11 +613,11 @@ public Optional<FieldChange> setField(Field field, String value, EntriesEventSou
         }
 
         String oldValue = getField(field).orElse(null);
-        boolean isNewField = oldValue == null;
         if (value.equals(oldValue)) {
             return Optional.empty();
         }
 
+        boolean isNewField = oldValue == null;
         changed = true;
 
         invalidateFieldCache(field);
@@ -1062,15 +1062,15 @@ public BibEntry withFiles(List<LinkedFile> files) {
      * Gets a list of linked files.
      *
      * @return the list of linked files, is never null but can be empty.
-     * Changes to the underlying list will have no effect on the entry itself. Use {@link #addFile(LinkedFile)}
+     * Changes to the underlying list will have no effect on the entry itself. Use {@link #addFile(LinkedFile)}.
      */
     public List<LinkedFile> getFiles() {
-        // Extract the path
         Optional<String> oldValue = getField(StandardField.FILE);
         if (oldValue.isEmpty()) {
             return new ArrayList<>(); // Return new ArrayList because emptyList is immutable
         }
 
+        // Extract the path
         return FileFieldParser.parse(oldValue.get());
     }
 

src/main/resources/l10n/JabRef_en.properties

@@ -276,7 +276,8 @@ Donate\ to\ JabRef=Donate to JabRef
 
 Download\ file=Download file
 
-Downloaded\ website\ as\ an\ HTML\ file.=Downloaded website as an HTML file.
+Download\ '%0'\ was\ a\ HTML\ file.\ Removed.=Download '%0' was a HTML file. Removed.
+Download\ '%0'\ was\ a\ HTML\ file.\ Keeping\ URL.=Download '%0' was a HTML file. Keeping URL.
 
 duplicate\ removal=duplicate removal
 
@@ -2402,7 +2403,6 @@ Convert\ timestamp\ field\ to\ field\ 'modificationdate'=Convert timestamp field
 New\ entry\ by\ type=New entry by type
 
 File\ '%1'\ is\ a\ duplicate\ of\ '%0'.\ Keeping\ '%0'=File '%1' is a duplicate of '%0'. Keeping '%0'
-File\ '%1'\ is\ a\ duplicate\ of\ '%0'.\ Keeping\ both\ due\ to\ deletion\ error=File '%1' is a duplicate of '%0'. Keeping both due to deletion error
 
 Enable\ field\ formatters=Enable field formatters
 Entry\ Type=Entry Type