Dd2480 2021 group 22 fix for issue 7452 (JabRef#7534)

* Ignore mime type params (JabRef#7452) Adds ignore parameter to mime type and notify the user if the file downloaded is HTML. * Update status bar message (JabRef#7452) Use Localization when writing messages in the status bar. * Add debug message (JabRef#7452) * Replace apache StringUtils (JabRef#7452) replace StringUtils::substringBefore with String::substring * Add UI test (JabRef#7452) Add test ensuring the UI warns the user if they download a linked HTML file (i.e. a web page). * Add unit test for HTML file (JabRef#7452) The test checks the resulting file type when downloading a HTML file. * Fix mime type parsing bug (JabRef#7452) Add check to only process the mimeType if an ';' exists inside the string * Add unit test for mime type parsing (JabRef#7452) Tests that mime type with parameter value is parsed correctly to exclude the parameter. * Add changes to changelog (JabRef#7452) * Clarify changes (JabRef#7452) * Reset cookie policy in test (JabRef#7452) Sets a new system-wide cookie manager if there is none, and sets the cookie policy to ACCEPT_NONE after each test. * Add test for when a linked file points to a PDF url (JabRef#7452) * Clean up code style (JabRef#7452) Clean up code styling according to JabRef style guidelines. * Remove duplicate changelog entry (JabRef#7452) * Refactor LinkedFileViewModelTest adding mock for JabRefPreferences used in testing (JabRef#7452) * Refactor LinkedFileViewModelTest removing redundant code (JabRef#7452) * Refactor LinkedFileViewModelTest removing redundant code (JabRef#7452) * Fix tests * fix checkstyle Co-authored-by: Binxin <binxin@kth.se> Co-authored-by: kittyt <kittyt@kth.se> Co-authored-by: Keivan Matinzadeh <matinzadeh.keivan@gmail.com> Co-authored-by: Johan Grundberg <johan.grundberg98@gmail.com> Co-authored-by: Johan Grundberg <grundb@kth.se> Co-authored-by: kaniyi <kaniyi@kth.se>
XDZhelheim · Mar 14, 2021 · 42270fa · 42270fa
1 parent 198572c
commit 42270fa
Show file tree

Hide file tree

Showing 5 changed files with 125 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -64,6 +64,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue where drag and drop of bib files for opening resulted in uncaught exceptions [#7464](https://github.com/JabRef/jabref/issues/7464)
 - We fixed an issue where columns shrink in width when we try to enlarge JabRef window. [#6818](https://github.com/JabRef/jabref/issues/6818)
 - We fixed an issue where Content selector does not seem to work for custom fields. [#6819](https://github.com/JabRef/jabref/issues/6819)
+- We fixed an issue in which a linked online file consisting of a web page was saved as an invalid pdf file upon being downloaded. The user is now notified when downloading a linked file results in an HTML file. [#7452](https://github.com/JabRef/jabref/issues/7452)
 
 ### Removed
 

diff --git a/src/main/java/org/jabref/gui/externalfiletype/ExternalFileTypes.java b/src/main/java/org/jabref/gui/externalfiletype/ExternalFileTypes.java
@@ -111,6 +111,10 @@ public Optional<ExternalFileType> getExternalFileTypeForName(String filename) {
      *         guaranteed to be returned.
      */
     public Optional<ExternalFileType> getExternalFileTypeByMimeType(String mimeType) {
+        // Ignores parameters according to link: (https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types)
+        if (mimeType.indexOf(';') != -1) {
+            mimeType = mimeType.substring(0, mimeType.indexOf(';')).trim();
+        }
         for (ExternalFileType type : externalFileTypes) {
             if (type.getMimeType().equalsIgnoreCase(mimeType)) {
                 return Optional.of(type);

diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
@@ -446,6 +446,11 @@ public void download() {
                     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);
+                }
             });
             downloadProgress.bind(downloadTask.workDonePercentageProperty());
             downloadTask.titleProperty().set(Localization.lang("Downloading"));

diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties
@@ -252,6 +252,9 @@ Do\ not\ write\ the\ following\ fields\ to\ XMP\ Metadata=Do not write the follo
 Donate\ to\ JabRef=Donate to JabRef
 
 Download\ file=Download file
+
+Downloaded\ website\ as\ an\ HTML\ file.=Downloaded website as an HTML file.
+
 duplicate\ removal=duplicate removal
 
 Duplicate\ string\ name=Duplicate string name

diff --git a/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java b/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java
@@ -1,17 +1,22 @@
 package org.jabref.gui.fieldeditors;
 
+import java.net.CookieHandler;
+import java.net.CookieManager;
+import java.net.CookiePolicy;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
+import java.util.List;
 import java.util.Optional;
 import java.util.TreeSet;
 
 import javafx.scene.control.Alert.AlertType;
 import javafx.scene.control.ButtonType;
 
 import org.jabref.gui.DialogService;
+import org.jabref.gui.externalfiletype.ExternalFileType;
 import org.jabref.gui.externalfiletype.ExternalFileTypes;
 import org.jabref.gui.externalfiletype.StandardExternalFileType;
 import org.jabref.gui.util.BackgroundTask;
@@ -24,6 +29,7 @@
 import org.jabref.model.entry.LinkedFile;
 import org.jabref.preferences.FilePreferences;
 
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -32,11 +38,15 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.contains;
+import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
@@ -51,6 +61,7 @@ class LinkedFileViewModelTest {
     private final ExternalFileTypes externalFileType = mock(ExternalFileTypes.class);
     private final FilePreferences filePreferences = mock(FilePreferences.class);
     private final XmpPreferences xmpPreferences = mock(XmpPreferences.class);
+    private CookieManager cookieManager;
 
     @BeforeEach
     void setUp(@TempDir Path tempFolder) throws Exception {
@@ -62,9 +73,25 @@ void setUp(@TempDir Path tempFolder) throws Exception {
 
         when(externalFileType.getExternalFileTypeSelection()).thenReturn(new TreeSet<>(ExternalFileTypes.getDefaultExternalFileTypes()));
         when(externalFileType.getExternalFileTypeByMimeType("application/pdf")).thenReturn(Optional.of(StandardExternalFileType.PDF));
+        when(externalFileType.getExternalFileTypeByMimeType(contains("text/html"))).thenReturn(Optional.of(StandardExternalFileType.URL));
         when(externalFileType.getExternalFileTypeByExt("pdf")).thenReturn(Optional.of(StandardExternalFileType.PDF));
+        when(externalFileType.getExternalFileTypeByExt("html")).thenReturn(Optional.of(StandardExternalFileType.URL));
         tempFile = tempFolder.resolve("temporaryFile");
         Files.createFile(tempFile);
+
+        // Check if there exists a system wide cookie handler
+        if (CookieHandler.getDefault() == null) {
+            cookieManager = new CookieManager();
+            CookieHandler.setDefault(cookieManager);
+        } else {
+            cookieManager = (CookieManager) CookieHandler.getDefault();
+        }
+        cookieManager.setCookiePolicy(CookiePolicy.ACCEPT_ALL);
+    }
+
+    @AfterEach
+    void tearDown() {
+        cookieManager.setCookiePolicy(CookiePolicy.ACCEPT_NONE);
     }
 
     @Test
@@ -151,24 +178,73 @@ void deleteWhenDialogCancelledReturnsFalseAndDoesNotRemoveFile() {
         assertTrue(Files.exists(tempFile));
     }
 
+    @Test
+    void downloadHtmlFileCausesWarningDisplay() throws MalformedURLException {
+        when(filePreferences.shouldStoreFilesRelativeToBib()).thenReturn(true);
+        when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("[entrytype]");
+        databaseContext.setDatabasePath(tempFile);
+
+        URL url = new URL("https://www.google.com/");
+        String fileType = StandardExternalFileType.URL.getName();
+        linkedFile = new LinkedFile(url, fileType);
+
+        LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, xmpPreferences, filePreferences, externalFileType);
+
+        viewModel.download();
+
+        verify(dialogService, atLeastOnce()).notify("Downloaded website as an HTML file.");
+    }
+
+    @Test
     void downloadDoesNotOverwriteFileTypeExtension() throws MalformedURLException {
         linkedFile = new LinkedFile(new URL("http://arxiv.org/pdf/1207.0408v1"), "");
 
         databaseContext = mock(BibDatabaseContext.class);
         when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("");
 
         LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, xmpPreferences, filePreferences, externalFileType);
 
         BackgroundTask<Path> task = viewModel.prepareDownloadTask(tempFile.getParent(), new URLDownload("http://arxiv.org/pdf/1207.0408v1"));
         task.onSuccess(destination -> {
             LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, Collections.singletonList(tempFile.getParent()), externalFileType);
-            assertEquals("asdf.PDF", newLinkedFile.getLink());
+            assertEquals("asdf.pdf", newLinkedFile.getLink());
             assertEquals("PDF", newLinkedFile.getFileType());
         });
         task.onFailure(Assertions::fail);
         new CurrentThreadTaskExecutor().execute(task);
     }
 
+    @Test
+    void downloadHtmlWhenLinkedFilePointsToHtml() throws MalformedURLException {
+        // the link mentioned in issue #7452
+        String url = "https://onlinelibrary.wiley.com/doi/abs/10.1002/0470862106.ia615";
+        String fileType = StandardExternalFileType.URL.getName();
+        linkedFile = new LinkedFile(new URL(url), fileType);
+
+        when(filePreferences.shouldStoreFilesRelativeToBib()).thenReturn(true);
+        when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("[entrytype]");
+
+        databaseContext.setDatabasePath(tempFile);
+
+        LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, xmpPreferences, filePreferences, externalFileType);
+
+        viewModel.download();
+
+        List<LinkedFile> linkedFiles = entry.getFiles();
+
+        for (LinkedFile file: linkedFiles) {
+            if (file.getLink().equalsIgnoreCase("Misc/asdf.html")) {
+                assertEquals("URL", file.getFileType());
+                return;
+            }
+        }
+        // If the file was not found among the linked files to the entry
+        fail();
+    }
+
     @Test
     void isNotSamePath() {
         linkedFile = new LinkedFile("desc", tempFile, "pdf");
@@ -190,4 +266,39 @@ void isSamePath() {
         LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, xmpPreferences, filePreferences, externalFileType);
         assertTrue(viewModel.isGeneratedPathSameAsOriginal());
     }
+
+
+    // Tests if added parameters to mimeType gets parsed to correct format.
+    @Test
+    void mimeTypeStringWithParameterIsReturnedAsWithoutParameter() {
+        Optional<ExternalFileType> test = externalFileType.getExternalFileTypeByMimeType("text/html; charset=UTF-8");
+        String actual = test.get().toString();
+        assertEquals("URL", actual);
+    }
+
+    @Test
+    void downloadPdfFileWhenLinkedFilePointsToPdfUrl() throws MalformedURLException {
+        linkedFile = new LinkedFile(new URL("http://arxiv.org/pdf/1207.0408v1"), "pdf");
+        // Needed Mockito stubbing methods to run test
+        when(filePreferences.shouldStoreFilesRelativeToBib()).thenReturn(true);
+        when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("[entrytype]");
+
+        databaseContext.setDatabasePath(tempFile);
+
+        LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, xmpPreferences, filePreferences, externalFileType);
+        viewModel.download();
+
+        // Loop through downloaded files to check for filetype='pdf'
+        List<LinkedFile> linkedFiles = entry.getFiles();
+        for (LinkedFile files : linkedFiles) {
+            if (files.getLink().equalsIgnoreCase("Misc/asdf.pdf")) {
+                assertEquals("pdf", files.getFileType().toLowerCase());
+                return;
+            }
+        }
+        // Assert fail if no PDF type was found
+        fail();
+    }
+
 }