From d3ce2e21dd78d6910c00acf8053134fa791e0970 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 11 May 2019 17:39:17 +0200 Subject: [PATCH 1/4] Fix downloading pdf produces html as extension If we already have a filetype we use that instead of relying on the autodetection Fixes #4913 --- .../jabref/gui/fieldeditors/LinkedFileViewModel.java | 2 +- .../gui/filelist/LinkedFilesEditDialogViewModel.java | 4 +--- .../logic/externalfiles/LinkedFileHandler.java | 12 +++++++++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java index 3fd76b4b379..b54ecaac34e 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java @@ -420,7 +420,7 @@ public BackgroundTask prepareDownloadTask(Path targetDirectory, URLDownloa String suggestedTypeName = suggestedType.map(ExternalFileType::getName).orElse(""); linkedFile.setFileType(suggestedTypeName); - String suggestedName = linkedFileHandler.getSuggestedFileName(); + String suggestedName = linkedFileHandler.getSuggestedFileName(suggestedTypeName); return targetDirectory.resolve(suggestedName); }) .then(destination -> new FileDownloadTask(urlDownload.getSource(), destination)) diff --git a/src/main/java/org/jabref/gui/filelist/LinkedFilesEditDialogViewModel.java b/src/main/java/org/jabref/gui/filelist/LinkedFilesEditDialogViewModel.java index 96005fc6b57..004c48273b5 100644 --- a/src/main/java/org/jabref/gui/filelist/LinkedFilesEditDialogViewModel.java +++ b/src/main/java/org/jabref/gui/filelist/LinkedFilesEditDialogViewModel.java @@ -91,9 +91,7 @@ public void openBrowseDialog() { public void setValues(LinkedFile linkedFile) { description.set(linkedFile.getDescription()); - - Path linkPath = Paths.get(linkedFile.getLink()); - link.set(relativize(linkPath)); + link.setValue(linkedFile.getLink()); //Might be an URL selectedExternalFileType.setValue(null); diff --git a/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java b/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java index 04c26ab179e..4aaed7c5498 100644 --- a/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java +++ b/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java @@ -19,6 +19,7 @@ import org.slf4j.LoggerFactory; public class LinkedFileHandler { + private static final Logger LOGGER = LoggerFactory.getLogger(LinkedFileHandler.class); private final BibDatabaseContext databaseContext; @@ -84,7 +85,7 @@ public boolean renameToName(String targetFileName) throws IOException { String expandedOldFilePath = oldFile.get().toString(); boolean pathsDifferOnlyByCase = newPath.toString().equalsIgnoreCase(expandedOldFilePath) - && !newPath.toString().equals(expandedOldFilePath); + && !newPath.toString().equals(expandedOldFilePath); if (Files.exists(newPath) && !pathsDifferOnlyByCase) { // We do not overwrite files @@ -113,9 +114,14 @@ private String relativize(Path path) { public String getSuggestedFileName() { String oldFileName = fileEntry.getLink(); + String extension = FileHelper.getFileExtension(oldFileName).orElse(fileEntry.getFileType()); + return getSuggestedFileName(extension); + } + + public String getSuggestedFileName(String extension) { String targetFileName = FileUtil.createFileNameFromPattern(databaseContext.getDatabase(), entry, filePreferences.getFileNamePattern()).trim() - + '.' - + FileHelper.getFileExtension(oldFileName).orElse(fileEntry.getFileType()); + + '.' + + extension; // Only create valid file names return FileUtil.getValidFileName(targetFileName); From bd016c138447602b13a48417614e770df74e41f9 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 17 May 2019 18:29:28 +0200 Subject: [PATCH 2/4] add relativze if not an URL --- .../filelist/LinkedFilesEditDialogViewModel.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jabref/gui/filelist/LinkedFilesEditDialogViewModel.java b/src/main/java/org/jabref/gui/filelist/LinkedFilesEditDialogViewModel.java index 004c48273b5..bfa7eb0ee97 100644 --- a/src/main/java/org/jabref/gui/filelist/LinkedFilesEditDialogViewModel.java +++ b/src/main/java/org/jabref/gui/filelist/LinkedFilesEditDialogViewModel.java @@ -76,9 +76,9 @@ public void openBrowseDialog() { String fileName = Paths.get(fileText).getFileName().toString(); FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder() - .withInitialDirectory(workingDir) - .withInitialFileName(fileName) - .build(); + .withInitialDirectory(workingDir) + .withInitialFileName(fileName) + .build(); dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(path -> { // Store the directory for next time: @@ -91,7 +91,12 @@ public void openBrowseDialog() { public void setValues(LinkedFile linkedFile) { description.set(linkedFile.getDescription()); - link.setValue(linkedFile.getLink()); //Might be an URL + + if (linkedFile.isOnlineLink()) { + link.setValue(linkedFile.getLink()); //Might be an URL + } else { + link.setValue(relativize(Paths.get(linkedFile.getLink()))); + } selectedExternalFileType.setValue(null); From c21c0f8d3f4a519841ce818ba1b8a2f866bf4fa4 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 17 May 2019 20:03:23 +0200 Subject: [PATCH 3/4] Create Test for download pdf document heavy mocking and refactoring of ExternalFileType TODO : Cleanup --- .../gui/externalfiles/FindFullTextAction.java | 5 +- .../gui/fieldeditors/LinkedFileViewModel.java | 11 +- .../LinkedFilesEditorViewModel.java | 19 +-- .../jabref/gui/filelist/AttachFileAction.java | 3 +- .../gui/maintable/MainTableColumnFactory.java | 5 +- .../fieldeditors/LinkedFileViewModelTest.java | 109 ++++++++++++------ 6 files changed, 101 insertions(+), 51 deletions(-) diff --git a/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java b/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java index 72c2bedd70d..d8ac847130e 100644 --- a/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java +++ b/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java @@ -13,6 +13,7 @@ import org.jabref.gui.BasePanel; import org.jabref.gui.DialogService; import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.externalfiletype.ExternalFileTypes; import org.jabref.gui.fieldeditors.LinkedFileViewModel; import org.jabref.gui.fieldeditors.LinkedFilesEditorViewModel; import org.jabref.gui.util.BackgroundTask; @@ -136,7 +137,7 @@ private void addLinkedFileFromURL(URL url, BibEntry entry, Path targetDirectory) basePanel.getBibDatabaseContext(), Globals.TASK_EXECUTOR, dialogService, - JabRefPreferences.getInstance()); + JabRefPreferences.getInstance(), ExternalFileTypes.getInstance()); try { URLDownload urlDownload = new URLDownload(newLinkedFile.getLink()); @@ -144,7 +145,7 @@ private void addLinkedFileFromURL(URL url, BibEntry entry, Path targetDirectory) downloadTask.onSuccess(destination -> { LinkedFile downloadedFile = LinkedFilesEditorViewModel.fromFile( destination, - basePanel.getBibDatabaseContext().getFileDirectoriesAsPaths(JabRefPreferences.getInstance().getFilePreferences())); + basePanel.getBibDatabaseContext().getFileDirectoriesAsPaths(JabRefPreferences.getInstance().getFilePreferences()), ExternalFileTypes.getInstance()); entry.addFile(downloadedFile); dialogService.notify(Localization.lang("Finished downloading full text document for entry %0.", entry.getCiteKeyOptional().orElse(Localization.lang("undefined")))); diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java index b54ecaac34e..f6ab783c26c 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java @@ -66,13 +66,15 @@ public class LinkedFileViewModel extends AbstractViewModel { private final FilePreferences filePreferences; private final XmpPreferences xmpPreferences; private final LinkedFileHandler linkedFileHandler; + private final ExternalFileTypes externalFileTypes; public LinkedFileViewModel(LinkedFile linkedFile, BibEntry entry, BibDatabaseContext databaseContext, TaskExecutor taskExecutor, DialogService dialogService, - JabRefPreferences preferences) { + JabRefPreferences preferences, + ExternalFileTypes externalFileTypes){ this.linkedFile = linkedFile; this.filePreferences = preferences.getFilePreferences(); @@ -81,6 +83,7 @@ public LinkedFileViewModel(LinkedFile linkedFile, this.entry = entry; this.dialogService = dialogService; this.taskExecutor = taskExecutor; + this.externalFileTypes = externalFileTypes; xmpPreferences = preferences.getXMPPreferences(); downloadOngoing.bind(downloadProgress.greaterThanOrEqualTo(0).and(downloadProgress.lessThan(1))); @@ -402,7 +405,7 @@ public void download() { URLDownload urlDownload = new URLDownload(linkedFile.getLink()); BackgroundTask downloadTask = prepareDownloadTask(targetDirectory.get(), urlDownload); downloadTask.onSuccess(destination -> { - LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectoriesAsPaths(filePreferences)); + LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectoriesAsPaths(filePreferences), externalFileTypes); linkedFile.setLink(newLinkedFile.getLink()); linkedFile.setFileType(newLinkedFile.getFileType()); }); @@ -443,7 +446,7 @@ private Optional inferFileTypeFromMimeType(URLDownload urlDown if (mimeType != null) { LOGGER.debug("MIME Type suggested: " + mimeType); - return ExternalFileTypes.getInstance().getExternalFileTypeByMimeType(mimeType); + return externalFileTypes.getExternalFileTypeByMimeType(mimeType); } else { return Optional.empty(); } @@ -451,7 +454,7 @@ private Optional inferFileTypeFromMimeType(URLDownload urlDown private Optional inferFileTypeFromURL(String url) { return URLUtil.getSuffix(url) - .flatMap(extension -> ExternalFileTypes.getInstance().getExternalFileTypeByExt(extension)); + .flatMap(extension -> externalFileTypes.getExternalFileTypeByExt(extension)); } public LinkedFile getFile() { diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java index 815d4be3a04..fa249881ab3 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java @@ -48,6 +48,7 @@ public class LinkedFilesEditorViewModel extends AbstractEditorViewModel { private final BibDatabaseContext databaseContext; private final TaskExecutor taskExecutor; private final JabRefPreferences preferences; + private final ExternalFileTypes externalFileTypes = ExternalFileTypes.getInstance(); public LinkedFilesEditorViewModel(String fieldName, AutoCompleteSuggestionProvider suggestionProvider, DialogService dialogService, @@ -86,9 +87,9 @@ private static String getStringRepresentation(List files) { * * TODO: Move this method to {@link LinkedFile} as soon as {@link CustomExternalFileType} lives in model. */ - public static LinkedFile fromFile(Path file, List fileDirectories) { + public static LinkedFile fromFile(Path file, List fileDirectories, ExternalFileTypes externalFileTypesFile) { String fileExtension = FileHelper.getFileExtension(file).orElse(""); - ExternalFileType suggestedFileType = ExternalFileTypes.getInstance() + ExternalFileType suggestedFileType = externalFileTypesFile .getExternalFileTypeByExt(fileExtension) .orElse(new UnknownExternalFileType(fileExtension)); Path relativePath = FileUtil.relativize(file, fileDirectories); @@ -98,8 +99,8 @@ public static LinkedFile fromFile(Path file, List fileDirectories) { public LinkedFileViewModel fromFile(Path file) { List fileDirectories = databaseContext.getFileDirectoriesAsPaths(preferences.getFilePreferences()); - LinkedFile linkedFile = fromFile(file, fileDirectories); - return new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences); + LinkedFile linkedFile = fromFile(file, fileDirectories, externalFileTypes); + return new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileTypes); } @@ -113,7 +114,7 @@ public BooleanProperty fulltextLookupInProgressProperty() { private List parseToFileViewModel(String stringValue) { return FileFieldParser.parse(stringValue).stream() - .map(linkedFile -> new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences)) + .map(linkedFile -> new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileTypes)) .collect(Collectors.toList()); } @@ -135,8 +136,8 @@ public void addNewFile() { List fileDirectories = databaseContext.getFileDirectoriesAsPaths(preferences.getFilePreferences()); dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(newFile -> { - LinkedFile newLinkedFile = fromFile(newFile, fileDirectories); - files.add(new LinkedFileViewModel(newLinkedFile, entry, databaseContext, taskExecutor, dialogService, preferences)); + LinkedFile newLinkedFile = fromFile(newFile, fileDirectories, externalFileTypes); + files.add(new LinkedFileViewModel(newLinkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileTypes)); }); } @@ -162,7 +163,7 @@ private List findAssociatedNotLinkedFiles(BibEntry entry) { try { List linkedFiles = util.findAssociatedNotLinkedFiles(entry); for (LinkedFile linkedFile : linkedFiles) { - LinkedFileViewModel newLinkedFile = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences); + LinkedFileViewModel newLinkedFile = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileTypes); newLinkedFile.markAsAutomaticallyFound(); result.add(newLinkedFile); } @@ -205,7 +206,7 @@ public void addFromURL() { } private void addFromURL(URL url) { - LinkedFileViewModel onlineFile = new LinkedFileViewModel(new LinkedFile(url, ""), entry, databaseContext, taskExecutor, dialogService, preferences); + LinkedFileViewModel onlineFile = new LinkedFileViewModel(new LinkedFile(url, ""), entry, databaseContext, taskExecutor, dialogService, preferences, externalFileTypes); files.add(onlineFile); onlineFile.download(); } diff --git a/src/main/java/org/jabref/gui/filelist/AttachFileAction.java b/src/main/java/org/jabref/gui/filelist/AttachFileAction.java index 8ff43c37060..46d8f565437 100644 --- a/src/main/java/org/jabref/gui/filelist/AttachFileAction.java +++ b/src/main/java/org/jabref/gui/filelist/AttachFileAction.java @@ -6,6 +6,7 @@ import org.jabref.gui.BasePanel; import org.jabref.gui.DialogService; import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.externalfiletype.ExternalFileTypes; import org.jabref.gui.fieldeditors.LinkedFilesEditorViewModel; import org.jabref.gui.undo.UndoableFieldChange; import org.jabref.gui.util.FileDialogConfiguration; @@ -38,7 +39,7 @@ public void execute() { dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(newFile -> { - LinkedFile linkedFile = LinkedFilesEditorViewModel.fromFile(newFile, panel.getBibDatabaseContext().getFileDirectoriesAsPaths(Globals.prefs.getFilePreferences())); + LinkedFile linkedFile = LinkedFilesEditorViewModel.fromFile(newFile, panel.getBibDatabaseContext().getFileDirectoriesAsPaths(Globals.prefs.getFilePreferences()), ExternalFileTypes.getInstance()); LinkedFileEditDialogView dialog = new LinkedFileEditDialogView(linkedFile); diff --git a/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java b/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java index 6133fc9c5e3..28ae799f41b 100644 --- a/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java +++ b/src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java @@ -65,6 +65,7 @@ class MainTableColumnFactory { private final CellFactory cellFactory; private final UndoManager undoManager; private final DialogService dialogService; + public MainTableColumnFactory(BibDatabaseContext database, ColumnPreferences preferences, ExternalFileTypes externalFileTypes, UndoManager undoManager, DialogService dialogService) { this.database = Objects.requireNonNull(database); @@ -264,7 +265,7 @@ private TableColumn> createFileColumn() .withOnMouseClickedEvent((entry, linkedFiles) -> event -> { if ((event.getButton() == MouseButton.PRIMARY) && (linkedFiles.size() == 1)) { // Only one linked file -> open directly - LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel(linkedFiles.get(0), entry.getEntry(), database, Globals.TASK_EXECUTOR, dialogService, Globals.prefs); + LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel(linkedFiles.get(0), entry.getEntry(), database, Globals.TASK_EXECUTOR, dialogService, Globals.prefs, externalFileTypes); linkedFileViewModel.open(); } }) @@ -287,7 +288,7 @@ private ContextMenu createFileMenu(BibEntryTableViewModel entry, List linkedFileViewModel.open()); diff --git a/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java b/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java index 7a7893c6d79..4a6ec55ae4d 100644 --- a/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java +++ b/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java @@ -1,14 +1,24 @@ package org.jabref.gui.fieldeditors; +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.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.ExternalFileTypes; +import org.jabref.gui.externalfiletype.StandardExternalFileType; +import org.jabref.gui.util.BackgroundTask; +import org.jabref.gui.util.CurrentThreadTaskExecutor; import org.jabref.gui.util.TaskExecutor; +import org.jabref.logic.externalfiles.LinkedFileHandler; +import org.jabref.logic.net.URLDownload; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.LinkedFile; @@ -20,6 +30,7 @@ import org.junit.jupiter.api.io.TempDir; import org.mockito.Answers; +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.mockito.ArgumentMatchers.any; @@ -39,14 +50,21 @@ class LinkedFileViewModelTest { private BibDatabaseContext databaseContext; private TaskExecutor taskExecutor; private DialogService dialogService; + private ExternalFileTypes externalFileType = mock(ExternalFileTypes.class); + private FilePreferences filePreferences = mock(FilePreferences.class); + private LinkedFileHandler linkedFileHandler = mock(LinkedFileHandler.class); @BeforeEach void setUp(@TempDir Path tempFolder) throws Exception { entry = new BibEntry(); + entry.setCiteKey("asdf"); databaseContext = new BibDatabaseContext(); taskExecutor = mock(TaskExecutor.class); dialogService = mock(DialogService.class); + when(externalFileType.getExternalFileTypeSelection()).thenReturn(new TreeSet<>(ExternalFileTypes.getDefaultExternalFileTypes())); + when(externalFileType.getExternalFileTypeByMimeType("application/pdf")).thenReturn(Optional.of(StandardExternalFileType.PDF)); + when(externalFileType.getExternalFileTypeByExt("pdf")).thenReturn(Optional.of(StandardExternalFileType.PDF)); tempFile = tempFolder.resolve("temporaryFile"); Files.createFile(tempFile); } @@ -57,7 +75,7 @@ void deleteWhenFilePathNotPresentReturnsTrue() { linkedFile = spy(new LinkedFile("", "nonexistent file", "")); doReturn(Optional.empty()).when(linkedFile).findIn(any(BibDatabaseContext.class), any(FilePreferences.class)); - LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences); + LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileType); boolean removed = viewModel.delete(); assertTrue(removed); @@ -68,14 +86,14 @@ void deleteWhenFilePathNotPresentReturnsTrue() { void deleteWhenRemoveChosenReturnsTrueButDoesNotDeletesFile() { linkedFile = new LinkedFile("", tempFile.toString(), ""); when(dialogService.showCustomButtonDialogAndWait( - any(AlertType.class), - anyString(), - anyString(), - any(ButtonType.class), - any(ButtonType.class), - any(ButtonType.class))).thenAnswer(invocation -> Optional.of(invocation.getArgument(3))); // first vararg - remove button - - LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences); + any(AlertType.class), + anyString(), + anyString(), + any(ButtonType.class), + any(ButtonType.class), + any(ButtonType.class))).thenAnswer(invocation -> Optional.of(invocation.getArgument(3))); // first vararg - remove button + + LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileType); boolean removed = viewModel.delete(); assertTrue(removed); @@ -86,14 +104,14 @@ void deleteWhenRemoveChosenReturnsTrueButDoesNotDeletesFile() { void deleteWhenDeleteChosenReturnsTrueAndDeletesFile() { linkedFile = new LinkedFile("", tempFile.toString(), ""); when(dialogService.showCustomButtonDialogAndWait( - any(AlertType.class), - anyString(), - anyString(), - any(ButtonType.class), - any(ButtonType.class), - any(ButtonType.class))).thenAnswer(invocation -> Optional.of(invocation.getArgument(4))); // second vararg - delete button - - LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences); + any(AlertType.class), + anyString(), + anyString(), + any(ButtonType.class), + any(ButtonType.class), + any(ButtonType.class))).thenAnswer(invocation -> Optional.of(invocation.getArgument(4))); // second vararg - delete button + + LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileType); boolean removed = viewModel.delete(); assertTrue(removed); @@ -104,14 +122,14 @@ void deleteWhenDeleteChosenReturnsTrueAndDeletesFile() { void deleteMissingFileReturnsTrue() { linkedFile = new LinkedFile("", "!!nonexistent file!!", ""); when(dialogService.showCustomButtonDialogAndWait( - any(AlertType.class), - anyString(), - anyString(), - any(ButtonType.class), - any(ButtonType.class), - any(ButtonType.class))).thenAnswer(invocation -> Optional.of(invocation.getArgument(4))); // second vararg - delete button - - LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences); + any(AlertType.class), + anyString(), + anyString(), + any(ButtonType.class), + any(ButtonType.class), + any(ButtonType.class))).thenAnswer(invocation -> Optional.of(invocation.getArgument(4))); // second vararg - delete button + + LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileType); boolean removed = viewModel.delete(); assertTrue(removed); @@ -121,17 +139,42 @@ void deleteMissingFileReturnsTrue() { void deleteWhenDialogCancelledReturnsFalseAndDoesNotRemoveFile() { linkedFile = new LinkedFile("desc", tempFile.toString(), "pdf"); when(dialogService.showCustomButtonDialogAndWait( - any(AlertType.class), - anyString(), - anyString(), - any(ButtonType.class), - any(ButtonType.class), - any(ButtonType.class))).thenAnswer(invocation -> Optional.of(invocation.getArgument(5))); // third vararg - cancel button - - LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences); + any(AlertType.class), + anyString(), + anyString(), + any(ButtonType.class), + any(ButtonType.class), + any(ButtonType.class))).thenAnswer(invocation -> Optional.of(invocation.getArgument(5))); // third vararg - cancel button + + LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileType); boolean removed = viewModel.delete(); assertFalse(removed); assertTrue(Files.exists(tempFile)); } + + @Test + void downloadDoesNotOverwriteFileTypeExtension() throws MalformedURLException { + linkedFile = new LinkedFile(new URL("http://arxiv.org/pdf/1207.0408v1"), ""); + + databaseContext = mock(BibDatabaseContext.class); + when(filePreferences.getFileNamePattern()).thenReturn("[bibtexkey]"); + when(databaseContext.getFirstExistingFileDir(any())).thenReturn(Optional.of(tempFile.getParent())); + when(databaseContext.getFileDirectoriesAsPaths(any())).thenReturn(Collections.singletonList(tempFile.getParent())); + when(preferences.getFilePreferences()).thenReturn(filePreferences); + when(linkedFileHandler.getSuggestedFileName("PDF")).thenReturn("asdf.pdf"); + LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, preferences, externalFileType); + + BackgroundTask task = viewModel.prepareDownloadTask(tempFile.getParent(), new URLDownload("http://arxiv.org/pdf/1207.0408v1")); + task.onSuccess(destination -> { + LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectoriesAsPaths(filePreferences), externalFileType); + + assertEquals("PDF", newLinkedFile.getFileType()); + + }); + new CurrentThreadTaskExecutor().execute(task); + + viewModel.download(); + + } } From adfa4a429097f5bea0c41546dfedee2761a8d082 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 18 May 2019 16:58:46 +0200 Subject: [PATCH 4/4] refactor and simply test fix cehckstyle fail test on exception --- .../gui/fieldeditors/LinkedFileViewModel.java | 2 +- .../LinkedFilesEditorViewModel.java | 2 +- .../fieldeditors/LinkedFileViewModelTest.java | 18 ++++++------------ 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java index f6ab783c26c..847847f836e 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java @@ -74,7 +74,7 @@ public LinkedFileViewModel(LinkedFile linkedFile, TaskExecutor taskExecutor, DialogService dialogService, JabRefPreferences preferences, - ExternalFileTypes externalFileTypes){ + ExternalFileTypes externalFileTypes) { this.linkedFile = linkedFile; this.filePreferences = preferences.getFilePreferences(); diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java index fa249881ab3..6b55626202f 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java @@ -56,6 +56,7 @@ public LinkedFilesEditorViewModel(String fieldName, AutoCompleteSuggestionProvid TaskExecutor taskExecutor, FieldCheckers fieldCheckers, JabRefPreferences preferences) { + super(fieldName, suggestionProvider, fieldCheckers); this.dialogService = dialogService; @@ -63,7 +64,6 @@ public LinkedFilesEditorViewModel(String fieldName, AutoCompleteSuggestionProvid this.taskExecutor = taskExecutor; this.preferences = preferences; - BindingsHelper.bindContentBidirectional( files, text, diff --git a/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java b/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java index 4a6ec55ae4d..fab6942fc8b 100644 --- a/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java +++ b/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java @@ -17,7 +17,6 @@ import org.jabref.gui.util.BackgroundTask; import org.jabref.gui.util.CurrentThreadTaskExecutor; import org.jabref.gui.util.TaskExecutor; -import org.jabref.logic.externalfiles.LinkedFileHandler; import org.jabref.logic.net.URLDownload; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; @@ -25,6 +24,7 @@ import org.jabref.model.metadata.FilePreferences; import org.jabref.preferences.JabRefPreferences; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -52,7 +52,6 @@ class LinkedFileViewModelTest { private DialogService dialogService; private ExternalFileTypes externalFileType = mock(ExternalFileTypes.class); private FilePreferences filePreferences = mock(FilePreferences.class); - private LinkedFileHandler linkedFileHandler = mock(LinkedFileHandler.class); @BeforeEach void setUp(@TempDir Path tempFolder) throws Exception { @@ -158,23 +157,18 @@ void downloadDoesNotOverwriteFileTypeExtension() throws MalformedURLException { linkedFile = new LinkedFile(new URL("http://arxiv.org/pdf/1207.0408v1"), ""); databaseContext = mock(BibDatabaseContext.class); - when(filePreferences.getFileNamePattern()).thenReturn("[bibtexkey]"); - when(databaseContext.getFirstExistingFileDir(any())).thenReturn(Optional.of(tempFile.getParent())); - when(databaseContext.getFileDirectoriesAsPaths(any())).thenReturn(Collections.singletonList(tempFile.getParent())); + when(filePreferences.getFileNamePattern()).thenReturn("[bibtexkey]"); //use this variant, as we cannot mock the linkedFileHandler cause it's initialized inside the viewModel when(preferences.getFilePreferences()).thenReturn(filePreferences); - when(linkedFileHandler.getSuggestedFileName("PDF")).thenReturn("asdf.pdf"); + LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, preferences, externalFileType); BackgroundTask task = viewModel.prepareDownloadTask(tempFile.getParent(), new URLDownload("http://arxiv.org/pdf/1207.0408v1")); task.onSuccess(destination -> { - LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectoriesAsPaths(filePreferences), externalFileType); - + LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, Collections.singletonList(tempFile.getParent()), externalFileType); + assertEquals("asdf.PDF", newLinkedFile.getLink()); assertEquals("PDF", newLinkedFile.getFileType()); - }); + task.onFailure(Assertions::fail); new CurrentThreadTaskExecutor().execute(task); - - viewModel.download(); - } }