From 2835cd5b0f7e0ca2ae00acb1b22fccd95fc1bf1c Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 2 Sep 2022 22:32:05 +0200 Subject: [PATCH] Refine code for BibEntry#replaceDownloadedFile (#9118) * Refine code for BibEntry#replaceDownloadedFile * Add tests * fix checkstyle Co-authored-by: Siedlerchr --- .../gui/fieldeditors/LinkedFileViewModel.java | 5 ++- .../model/database/BibDatabaseContext.java | 3 +- .../java/org/jabref/model/entry/BibEntry.java | 22 ++++++++++--- .../fieldeditors/LinkedFileViewModelTest.java | 31 ++++++++++++++++--- .../org/jabref/model/entry/BibEntryTest.java | 27 ++++++++-------- 5 files changed, 60 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java index 14fcf013668..65d5c505f75 100644 --- 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 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 index 662a1ecde36..c341d0e17bd 100644 --- 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 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 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 index 2643e777c84..63204636765 100644 --- 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 linkedFiles) { + /** + * Helper method to add a downloaded file to the entry. + *

+ * 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 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 index e11100032c4..a501f43ec88 100644 --- 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 index 7147ae78577..84ad88fff79 100644 --- 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 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(',');