Fix for issue 6197: Do not store downloaded file if it is already attached

CHANGELOG.md

@@ -90,6 +90,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue where the article title with colon fails to download the arXiv link (pdf file). [#7660](https://github.com/JabRef/issues/7660)
 - We fixed an issue where the keybinding for delete entry did not work on the main table [7580](https://github.com/JabRef/jabref/pull/7580)
 - We fixed an issue where the RFC fetcher is not compatible with the draft [7305](https://github.com/JabRef/jabref/issues/7305)
+-  We fixed an issue where duplicate files (both file names and contents are the same) is downloaded and add to linked files  [#6197](https://github.com/JabRef/jabref/issues/6197)
 
 ### Removed
 
@@ -580,4 +581,4 @@ The changelog of JabRef 2.11 and all previous versions is available as [text fil
 [5.0-beta]: https://github.com/JabRef/jabref/compare/v5.0-alpha...v5.0-beta
 [5.0-alpha]: https://github.com/JabRef/jabref/compare/v4.3...v5.0-alpha
 
-<!-- markdownlint-disable-file MD024 MD033 -->
+<!-- markdownlint-disable-file MD024 MD033 -->

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

@@ -434,29 +434,34 @@ public void download() {
 
             BackgroundTask<Path> downloadTask = prepareDownloadTask(targetDirectory.get(), urlDownload);
             downloadTask.onSuccess(destination -> {
-                LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectories(filePreferences), externalFileTypes);
-
-                List<LinkedFile> linkedFiles = entry.getFiles();
-                int oldFileIndex = -1;
-                int i = 0;
-                while ((i < linkedFiles.size()) && (oldFileIndex == -1)) {
-                    LinkedFile file = linkedFiles.get(i);
-                    // The file type changes as part of download process (see prepareDownloadTask), thus we only compare by link
-                    if (file.getLink().equalsIgnoreCase(linkedFile.getLink())) {
-                        oldFileIndex = i;
+                try {
+                    if (!FileNameUniqueness.isDuplicatedFile(targetDirectory.get().toString(), destination.getFileName().toString(), dialogService)) {
+                        LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectories(filePreferences), externalFileTypes);
+                        List<LinkedFile> linkedFiles = entry.getFiles();
+                        int oldFileIndex = -1;
+                        int i = 0;
+                        while ((i < linkedFiles.size()) && (oldFileIndex == -1)) {
+                            LinkedFile file = linkedFiles.get(i);
+                            // The file type changes as part of download process (see prepareDownloadTask), thus we only compare by link
+                            if (file.getLink().equalsIgnoreCase(linkedFile.getLink())) {
+                                oldFileIndex = i;
+                            }
+                            i++;
+                        }
+                        if (oldFileIndex == -1) {
+                            linkedFiles.add(0, newLinkedFile);
+                        } else {
+                            linkedFiles.set(oldFileIndex, newLinkedFile);
+                        }
+                        entry.setFiles(linkedFiles);
+                        // Notify in bar when the file type is HTML.
+                        if (newLinkedFile.getFileType().equals(StandardExternalFileType.URL.getName())) {
+                            dialogService.notify(Localization.lang("Downloaded website as an HTML file."));
+                            LOGGER.debug("Downloaded website {} as an HTML file at {}", linkedFile.getLink(), destination);
+                        }
                     }
-                    i++;
-                }
-                if (oldFileIndex == -1) {
-                    linkedFiles.add(0, newLinkedFile);
-                } else {
-                    linkedFiles.set(oldFileIndex, newLinkedFile);
-                }
-                entry.setFiles(linkedFiles);
-                // Notify in bar when the file type is HTML.
-                if (newLinkedFile.getFileType().equals(StandardExternalFileType.URL.getName())) {
-                    dialogService.notify(Localization.lang("Downloaded website as an HTML file."));
-                    LOGGER.debug("Downloaded website {} as an HTML file at {}", linkedFile.getLink(), destination);
+                } catch (IOException e) {
+                    e.printStackTrace();
                 }
             });
             downloadProgress.bind(downloadTask.workDonePercentageProperty());

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

@@ -1,9 +1,14 @@
 package org.jabref.logic.util.io;
 
+import java.io.File;
+import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Optional;
 
+import org.jabref.gui.DialogService;
+import org.jabref.logic.l10n.Localization;
+
 public class FileNameUniqueness {
 
     /**
@@ -41,4 +46,91 @@ public static String getNonOverWritingFileName(Path targetDirectory, String file
 
         return newFileName;
     }
+
+    /**
+     *
+     * @param directory The directory which saves the files (.pdf, for example)
+     * @param fileName Suggest name for the newly downloaded file
+     * @param dialogService To display the error and success message
+     * @return true when the content of files is duplicate and successfully delete the newly downloaded file ,
+     *         false when it is not a duplicate file or fail to delete the duplicate file
+     * @throws IOException Fail when the file is not exist or something wrong when reading the file
+     */
+    public static boolean isDuplicatedFile(String directory, String fileName, DialogService dialogService) throws IOException {
+
+        Optional<String> extensionOptional = FileUtil.getFileExtension(fileName);
+        String extensionSuffix;
+        String fileNameWithoutDuplicated;
+
+        if (extensionOptional.isPresent()) {
+            extensionSuffix = '.' + extensionOptional.get();
+            fileNameWithoutDuplicated = eraseDuplicateMarks(fileName.substring(0, fileName.lastIndexOf('.')));
+        } else {
+            extensionSuffix = "";
+            fileNameWithoutDuplicated = eraseDuplicateMarks(fileName);
+        }
+
+        String originalFileName = fileNameWithoutDuplicated + extensionSuffix;
+        if (fileName.equals(originalFileName)) {
+            return false;
+        }
+
+        File originalFile = new File(directory, originalFileName);
+        // deal with a very special case, when duplication does not happen, but the file name is end with something like " (1)" as it originally is
+        if (!originalFile.exists()) {
+            return false;
+        }
+
+        File duplicateFile = new File(directory, fileName);
+        assert (duplicateFile.exists());
+        int counter = 1;
+        while (true) {
+            if (com.google.common.io.Files.equal(originalFile, duplicateFile)) {
+                if (duplicateFile.delete()) {
+                    dialogService.notify(Localization.lang("Dupilcate file with '%0', succesfully delete the file '%1'", originalFileName, fileName));
+                } else {
+                    dialogService.notify(Localization.lang("Dupilcate file with '%0', fail to delete the file '%1'", originalFileName, fileName));
+                }
+                return true;
+            }
+
+            originalFileName = fileNameWithoutDuplicated +
+                    " (" + counter + ")"
+                    + extensionSuffix;
+            counter++;
+
+            if (originalFileName.equals(fileName)) {
+                dialogService.notify(Localization.lang("Duplicate file name but different content, keep the new file '%0'", fileName));
+                return false;
+            }
+
+            originalFile = new File(directory, originalFileName);
+            if (!originalFile.exists()) {
+                return false;
+            }
+        }
+    }
+
+    /**
+     * recover the file name to origin if it has duplicate mark such as " (1)"
+     * It will change the String whose format is "xxxxxx (number)" into "xxxxxx", while return the same String when it does not match the format
+     *
+     * @param fileName Suggested name for the file without extensionSuffix, if it has duplicate file name with other file, it will end with something like " (1)"
+     * @return Suggested name for the file without extensionSuffix and duplicate marks such as " (1)"
+     */
+    public static String eraseDuplicateMarks(String fileName) {
+        int dotPosition1 = fileName.lastIndexOf(')');
+        if (dotPosition1 != fileName.length() - 1 || dotPosition1 <= 2) {
+            return fileName;
+        } else {
+            if (!Character.isDigit(fileName.charAt(dotPosition1 - 1))) {
+                return fileName;
+            }
+        }
+        int dotPosition = fileName.lastIndexOf('(');
+        if ((dotPosition > 0) && (dotPosition < (fileName.length() - 1))) {
+            fileName = fileName.substring(0, dotPosition - 1);
+        }
+        return fileName;
+    }
 }

src/test/java/org/jabref/logic/util/io/FileNameUniquenessTest.java

@@ -44,4 +44,32 @@ public void testGetNonOverWritingFileNameReturnsUniqueNameOverNConflicts() throw
         String outputFileName = FileNameUniqueness.getNonOverWritingFileName(tempDir, "manyfiles.txt");
         assertEquals("manyfiles (2).txt", outputFileName);
     }
+
+    @Test
+    public void testTaseDuplicateMarksReturnsOrignalFileName1() throws IOException {
+        String fileName1 = "abc def (1)";
+        String fileName2 = FileNameUniqueness.eraseDuplicateMarks(fileName1);
+        assertEquals("abc def", fileName2);
+    }
+
+    @Test
+    public void testTaseDuplicateMarksReturnsOrignalFileName2() throws IOException {
+        String fileName1 = "abc (def) gh (1)";
+        String fileName2 = FileNameUniqueness.eraseDuplicateMarks(fileName1);
+        assertEquals("abc (def) gh", fileName2);
+    }
+
+    @Test
+    public void testTaseDuplicateMarksReturnsSameName1() throws IOException {
+        String fileName1 = "abc def (g)";
+        String fileName2 = FileNameUniqueness.eraseDuplicateMarks(fileName1);
+        assertEquals("abc def (g)", fileName2);
+    }
+
+    @Test
+    public void testTaseDuplicateMarksReturnsSameName2() throws IOException {
+        String fileName1 = "abc def";
+        String fileName2 = FileNameUniqueness.eraseDuplicateMarks(fileName1);
+        assertEquals("abc def", fileName2);
+    }
 }