Refine code for BibEntry#replaceDownloadedFile (#9118)

* Refine code for BibEntry#replaceDownloadedFile * Add tests * fix checkstyle Co-authored-by: Siedlerchr <siedlerkiller@gmail.com>
JabRef · Sep 2, 2022 · 2835cd5 · 2835cd5
1 parent 7239503
commit 2835cd5
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 28 deletions.
diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
@@ -454,13 +454,12 @@ public void download() {
                 }
 
                 if (!isDuplicate) {
+                    // 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(preferences.getFilePreferences()),
                             preferences.getFilePreferences());
-                    List<LinkedFile> linkedFiles = entry.getFiles();
-
-                    entry.addLinkedFile(entry, linkedFile, newLinkedFile, linkedFiles);
+                    entry.replaceDownloadedFile(linkedFile.getLink(), newLinkedFile);
 
                     // Notify in bar when the file type is HTML.
                     if (newLinkedFile.getFileType().equals(StandardExternalFileType.URL.getName())) {

diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java
@@ -160,8 +160,7 @@ public List<Path> getFileDirectories(FilePreferences preferences) {
     /**
      * Returns the first existing file directory from  {@link #getFileDirectories(FilePreferences)}
      *
-     * @param preferences The FilePreferences
-     * @return Optional of Path
+     * @return the path - or an empty optional, if none of the directories exists
      */
     public Optional<Path> getFirstExistingFileDir(FilePreferences preferences) {
         return getFileDirectories(preferences).stream().filter(Files::exists).findFirst();

diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java
@@ -1002,22 +1002,34 @@ public Observable[] getObservables() {
         return new Observable[] {fields, type};
     }
 
-    public void addLinkedFile(BibEntry entry, LinkedFile linkedFile, LinkedFile newLinkedFile, List<LinkedFile> linkedFiles) {
+    /**
+     * Helper method to add a downloaded file to the entry.
+     * <p>
+     * Use-case: URL is contained in the file, the file is downloaded and should then replace the url.
+     * This method. adds the given path (as file) to the entry and removes the url.
+     *
+     * @param linkToDownloadedFile the link to the file, which was downloaded
+     * @param downloadedFile the path to be added to the entry
+     */
+    public void replaceDownloadedFile(String linkToDownloadedFile, LinkedFile downloadedFile) {
+        List<LinkedFile> linkedFiles = this.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())) {
+            if (file.getLink().equalsIgnoreCase(linkToDownloadedFile)) {
                 oldFileIndex = i;
             }
             i++;
         }
         if (oldFileIndex == -1) {
-            linkedFiles.add(0, newLinkedFile);
+            linkedFiles.add(0, downloadedFile);
         } else {
-            linkedFiles.set(oldFileIndex, newLinkedFile);
+            linkedFiles.set(oldFileIndex, downloadedFile);
         }
-        entry.setFiles(linkedFiles);
+
+        this.setFiles(linkedFiles);
     }
 }
diff --git a/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java b/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java
@@ -8,6 +8,7 @@
 import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
@@ -68,8 +69,9 @@ class LinkedFileViewModelTest {
 
     @BeforeEach
     void setUp(@TempDir Path tempFolder) throws Exception {
-        entry = new BibEntry();
-        entry.setCitationKey("asdf");
+        entry = new BibEntry()
+                .withCitationKey("asdf");
+
         databaseContext = new BibDatabaseContext();
         taskExecutor = mock(TaskExecutor.class);
         dialogService = mock(DialogService.class);
@@ -80,7 +82,7 @@ void setUp(@TempDir Path tempFolder) throws Exception {
         tempFile = tempFolder.resolve("temporaryFile");
         Files.createFile(tempFile);
 
-        // Check if there exists a system wide cookie handler
+        // Check if there exists a system-wide cookie handler
         if (CookieHandler.getDefault() == null) {
             cookieManager = new CookieManager();
             CookieHandler.setDefault(cookieManager);
@@ -197,8 +199,27 @@ void downloadHtmlFileCausesWarningDisplay() throws MalformedURLException {
         verify(dialogService, atLeastOnce()).notify("Downloaded website as an HTML file.");
     }
 
+    @FetcherTest
+    @Test
+    void downloadOfFileReplacesLink(@TempDir Path tempFolder) throws Exception {
+        linkedFile = new LinkedFile(new URL("http://arxiv.org/pdf/1207.0408v1"), "");
+        entry.setFiles(new ArrayList<>(List.of(linkedFile)));
+
+        databaseContext = mock(BibDatabaseContext.class);
+        when(databaseContext.getFirstExistingFileDir(any())).thenReturn(Optional.of(tempFolder));
+
+        when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("");
+
+        LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, preferences);
+        viewModel.download();
+
+        assertEquals(List.of(new LinkedFile("", tempFolder.resolve("asdf.pdf"), "PDF")), entry.getFiles());
+    }
+
+    @FetcherTest
     @Test
-    void downloadDoesNotOverwriteFileTypeExtension() throws MalformedURLException {
+    void downloadDoesNotOverwriteFileTypeExtension() throws Exception {
         linkedFile = new LinkedFile(new URL("http://arxiv.org/pdf/1207.0408v1"), "");
 
         databaseContext = mock(BibDatabaseContext.class);
@@ -220,7 +241,7 @@ void downloadDoesNotOverwriteFileTypeExtension() throws MalformedURLException {
     @FetcherTest
     @Test
     void downloadHtmlWhenLinkedFilePointsToHtml() throws MalformedURLException {
-        // use google as test url, wiley is protected by clooudfare
+        // use google as test url, wiley is protected by CloudFlare
         String url = "https://google.com";
         String fileType = StandardExternalFileType.URL.getName();
         linkedFile = new LinkedFile(new URL(url), fileType);

diff --git a/src/test/java/org/jabref/model/entry/BibEntryTest.java b/src/test/java/org/jabref/model/entry/BibEntryTest.java
@@ -1,5 +1,6 @@
 package org.jabref.model.entry;
 
+import java.net.URL;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -19,8 +20,6 @@
 import org.jabref.model.entry.types.StandardEntryType;
 
 import com.google.common.collect.Sets;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.parallel.Execution;
 
@@ -34,17 +33,7 @@
 
 @Execution(CONCURRENT)
 class BibEntryTest {
-    private BibEntry entry;
-
-    @BeforeEach
-    void setUp() {
-        entry = new BibEntry();
-    }
-
-    @AfterEach
-    void tearDown() {
-        entry = null;
-    }
+    private BibEntry entry = new BibEntry();
 
     @Test
     void testDefaultConstructor() {
@@ -260,6 +249,18 @@ void testGetAndAddToLinkedFileList() {
         assertEquals(Arrays.asList(new LinkedFile("", Path.of(""), "")), entry.getFiles());
     }
 
+    @Test
+    void replaceOfLinkWorks() throws Exception {
+        List<LinkedFile> files = new ArrayList<>();
+        String urlAsString = "https://www.example.org/file.pdf";
+        files.add(new LinkedFile(new URL(urlAsString), ""));
+        entry.setFiles(files);
+
+        LinkedFile linkedFile = new LinkedFile("", Path.of("file.pdf", ""), "");
+        entry.replaceDownloadedFile(urlAsString, linkedFile);
+        assertEquals(List.of(linkedFile), entry.getFiles());
+    }
+
     @Test
     void testGetEmptyKeywords() {
         KeywordList actual = entry.getKeywords(',');