Merge branch 'fix-for-issue-7452' of https://github.com/DD2480-2021-g…

…roup-22/jabref into DD2480-2021-group-22-fix-for-issue-7452 * 'fix-for-issue-7452' of https://github.com/DD2480-2021-group-22/jabref: Refactor LinkedFileViewModelTest removing redundant code (#7452) Refactor LinkedFileViewModelTest removing redundant code (#7452) Refactor LinkedFileViewModelTest adding mock for JabRefPreferences used in testing (#7452) Remove duplicate changelog entry (#7452) Clean up code style (#7452) Clean up code styling according to JabRef style guidelines. Add test for when a linked file points to a PDF url (#7452) Reset cookie policy in test (#7452) Clarify changes (#7452) Add changes to changelog (#7452) Add unit test for mime type parsing (#7452) Fix mime type parsing bug (#7452) Add check to only process the mimeType if an ';' exists inside the string Add unit test for HTML file (#7452) Add UI test (#7452) Add test ensuring the UI warns the user if they download a linked HTML file (i.e. a web page). Replace apache StringUtils (#7452) replace StringUtils::substringBefore with String::substring Add debug message (#7452) Update status bar message (#7452) Ignore mime type params (#7452)
JabRef · Mar 14, 2021 · 7bd9c0d · 7bd9c0d
2 parents 198572c + 3a03b5c
commit 7bd9c0d
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 0 deletions.
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,23 @@
 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 java.util.regex.Pattern;
 
 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 +30,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 +39,16 @@
 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.ArgumentMatchers.matches;
+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 +63,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 +75,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,6 +180,28 @@ void deleteWhenDialogCancelledReturnsFalseAndDoesNotRemoveFile() {
         assertTrue(Files.exists(tempFile));
     }
 
+    // Structure taken from deleteWhenDialogCancelledReturnsFalseAndDoesNotRemoveFile method
+    @Test
+    void downloadHtmlFileCausesWarningDisplay() throws MalformedURLException {
+        // From BibDatabaseContextTest::setUp
+        when(filePreferences.shouldStoreFilesRelativeToBib()).thenReturn(true);
+        when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]"); // used in other tests in this class
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("[entrytype]"); // used in movesFileWithFileDirPattern at MoveFilesCleanupTest.java
+
+        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();
+
+        Pattern warningPattern = Pattern.compile(".*HTML.*", Pattern.CASE_INSENSITIVE);
+        verify(dialogService, atLeastOnce()).notify(matches(warningPattern));
+    }
+
     void downloadDoesNotOverwriteFileTypeExtension() throws MalformedURLException {
         linkedFile = new LinkedFile(new URL("http://arxiv.org/pdf/1207.0408v1"), "");
 
@@ -169,6 +220,35 @@ void downloadDoesNotOverwriteFileTypeExtension() throws MalformedURLException {
         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 +270,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();
+    }
+
 }