Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provides download option in context menu and fixes #3614 #3824

Merged
merged 6 commits into from
Mar 17, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ For more details refer to the [field mapping help page](http://help.jabref.org/e
- We added Facebook and Twitter icons in the toolbar to link to our [Facebook](https://www.facebook.com/JabRef/) and [Twitter](https://twitter.com/jabref_org) pages.
- Renamed the _Review_ Tab into _Comments_ Tab
- We no longer print empty lines when exporting an entry in RIS format [#3634](https://github.com/JabRef/jabref/issues/3634)
- We added the option to download linked URLs in the context menu in the entry editor.
- We improved file saving so that hard links are now preserved when a save is performed [#2633](https://github.com/JabRef/jabref/issues/2633)
- We changed the default dialog option when removing a [file link](http://help.jabref.org/en/FileLinks#adding-external-links-to-an-entry) from an entry.
The new default removes the linked file from the entry instead of deleting the file from disk. [#3679](https://github.com/JabRef/jabref/issues/3679)
Expand All @@ -45,6 +46,7 @@ The new default removes the linked file from the entry instead of deleting the f
- We fixed an issue where pressing space caused the cursor to jump to the start of the text field. [#3471](https://github.com/JabRef/jabref/issues/3471)
- We fixed the missing dot in the name of an exported file. [#3576](https://github.com/JabRef/jabref/issues/3576)
- Autocompletion in the search bar can now be disabled via the preferences. [#3598](https://github.com/JabRef/jabref/issues/3598)
- We fixed an issue where the progress of an ongoing file download was not shown correctly. [#3614](https://github.com/JabRef/jabref/issues/3614)
- We fixed an issue where odd linked files could not be selected in the entry editor. [#3639](https://github.com/JabRef/jabref/issues/3639)
- We fixed and extended the RIS import functionality to cover more fields. [#3634](https://github.com/JabRef/jabref/issues/3634) [#2607](https://github.com/JabRef/jabref/issues/2607)
- Chaining modifiers in BibTeX key pattern now works as described in the documentation. [#3648](https://github.com/JabRef/jabref/issues/3648)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ protected Void call() throws Exception {
EasyBind.subscribe(
inputStream.totalNumBytesReadProperty(),
bytesRead -> updateProgress(bytesRead.longValue(), inputStream.getMaxNumBytes()));

// Make sure directory exists since otherwise copy fails
Files.createDirectories(destination.getParent());

Files.copy(inputStream, destination, StandardCopyOption.REPLACE_EXISTING);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static FieldEditorFX getForField(String fieldName, TaskExecutor taskExecu
} else if (fieldExtras.contains(FieldProperty.OWNER)) {
return new OwnerEditor(fieldName, preferences, suggestionProvider, fieldCheckers);
} else if (fieldExtras.contains(FieldProperty.FILE_EDITOR)) {
return new LinkedFilesEditor(fieldName, dialogService, databaseContext, taskExecutor, suggestionProvider, fieldCheckers);
return new LinkedFilesEditor(fieldName, dialogService, databaseContext, taskExecutor, suggestionProvider, fieldCheckers, preferences);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do really the whole preferences need to be passed? Didn't we want to implement information hiding here? 😇

} else if (fieldExtras.contains(FieldProperty.YES_NO)) {
return new OptionEditor<>(new YesNoEditorViewModel(fieldName, suggestionProvider, fieldCheckers));
} else if (fieldExtras.contains(FieldProperty.MONTH)) {
Expand Down
126 changes: 121 additions & 5 deletions src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jabref.gui.fieldeditors;

import java.io.IOException;
import java.net.MalformedURLException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand All @@ -17,6 +18,7 @@
import javafx.beans.property.DoubleProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleDoubleProperty;
import javafx.beans.property.StringProperty;
import javafx.scene.control.Alert.AlertType;
import javafx.scene.control.ButtonType;

Expand All @@ -25,6 +27,8 @@
import org.jabref.gui.DialogService;
import org.jabref.gui.FXDialogService;
import org.jabref.gui.desktop.JabRefDesktop;
import org.jabref.gui.externalfiles.DownloadExternalFile;
import org.jabref.gui.externalfiles.FileDownloadTask;
import org.jabref.gui.externalfiletype.ExternalFileType;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.filelist.FileListEntryEditor;
Expand All @@ -33,12 +37,15 @@
import org.jabref.logic.cleanup.MoveFilesCleanup;
import org.jabref.logic.cleanup.RenamePdfCleanup;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.net.URLDownload;
import org.jabref.logic.util.OS;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.logic.xmp.XmpUtilWriter;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.metadata.FileDirectoryPreferences;
import org.jabref.preferences.JabRefPreferences;

import de.jensd.fx.glyphs.GlyphIcons;
import de.jensd.fx.glyphs.materialdesignicons.MaterialDesignIcon;
Expand Down Expand Up @@ -73,7 +80,7 @@ protected LinkedFileViewModel(LinkedFile linkedFile, BibEntry entry, BibDatabase
this.taskExecutor = taskExecutor;
this.dialogService = dialogService;

downloadOngoing.bind(downloadProgress.greaterThanOrEqualTo(0).and(downloadProgress.lessThan(100)));
downloadOngoing.bind(downloadProgress.greaterThanOrEqualTo(0).and(downloadProgress.lessThan(1)));
canWriteXMPMetadata.setValue(!linkedFile.isOnlineLink() && linkedFile.getFileType().equalsIgnoreCase("pdf"));
}

Expand All @@ -97,12 +104,12 @@ public DoubleProperty downloadProgressProperty() {
return downloadProgress;
}

public LinkedFile getFile() {
return linkedFile;
public StringProperty linkProperty() {
return linkedFile.linkProperty();
}

public String getLink() {
return linkedFile.getLink();
public StringProperty descriptionProperty() {
return linkedFile.descriptionProperty();
}

public String getDescription() {
Expand Down Expand Up @@ -345,4 +352,113 @@ public void writeXMPMetadata() {
// TODO: Show progress
taskExecutor.execute(writeTask);
}

public void download() {
if (!linkedFile.isOnlineLink()) {
throw new UnsupportedOperationException("In order to download the file it has to be an online link");
}

try {
URLDownload urlDownload = new URLDownload(linkedFile.getLink());
Optional<ExternalFileType> suggestedType = inferFileType(urlDownload);
String suggestedTypeName = suggestedType.map(ExternalFileType::getName).orElse("");
linkedFile.setFileType(suggestedTypeName);
List<Path> fileDirectories = databaseContext.getFileDirectoriesAsPaths(Globals.prefs.getFileDirectoryPreferences());

Path destination = constructSuggestedPath(suggestedType, fileDirectories);

BackgroundTask<Void> downloadTask = new FileDownloadTask(urlDownload.getSource(), destination)
.onSuccess(event -> {
LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, fileDirectories);
linkedFile.setLink(newLinkedFile.getLink());
linkedFile.setFileType(newLinkedFile.getFileType());
})
.onFailure(ex -> dialogService.showErrorDialogAndWait("", ex));

downloadProgress.bind(downloadTask.workDonePercentageProperty());
taskExecutor.execute(downloadTask);
} catch (MalformedURLException exception) {
dialogService.showErrorDialogAndWait(
Localization.lang("Invalid URL"),
exception);
}
}

private Path constructSuggestedPath(Optional<ExternalFileType> suggestedType, List<Path> fileDirectories) {
String suffix = suggestedType.map(ExternalFileType::getExtension).orElse("");
String suggestedName = getSuggestedFileName(suffix);
Path directory;
if (fileDirectories.isEmpty()) {
directory = null;
} else {
directory = fileDirectories.get(0);
}
final Path suggestDir = directory == null ? Paths.get(System.getProperty("user.home")) : directory;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a method which always returns a file dir: getfirstExistingFileDir

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good observation. Thanks for the hint.

return suggestDir.resolve(suggestedName);
}

private Optional<ExternalFileType> inferFileType(URLDownload urlDownload) {
Optional<ExternalFileType> suggestedType = inferFileTypeFromMimeType(urlDownload);

// If we did not find a file type from the MIME type, try based on extension:
if (!suggestedType.isPresent()) {
suggestedType = inferFileTypeFromURL(urlDownload.getSource().toExternalForm());
}
return suggestedType;
}

private Optional<ExternalFileType> inferFileTypeFromMimeType(URLDownload urlDownload) {
try {
// TODO: what if this takes long time?
String mimeType = urlDownload.getMimeType(); // Read MIME type
if (mimeType != null) {
LOGGER.debug("MIME Type suggested: " + mimeType);
return ExternalFileTypes.getInstance().getExternalFileTypeByMimeType(mimeType);
} else {
return Optional.empty();
}
} catch (IOException ex) {
LOGGER.debug("Error while inferring MIME type for URL " + urlDownload.getSource(), ex);
return Optional.empty();
}
}

private Optional<ExternalFileType> inferFileTypeFromURL(String url) {
String extension = DownloadExternalFile.getSuffix(url);
if (extension != null) {
return ExternalFileTypes.getInstance().getExternalFileTypeByExt(extension);
} else {
return Optional.empty();
}
}

private String getSuggestedFileName(String suffix) {
String plannedName = FileUtil.createFileNameFromPattern(databaseContext.getDatabase(), entry,
Globals.prefs.get(JabRefPreferences.IMPORT_FILENAMEPATTERN));

if (!suffix.isEmpty()) {
plannedName += "." + suffix;
}

/*
* [ 1548875 ] download pdf produces unsupported filename
*
* http://sourceforge.net/tracker/index.php?func=detail&aid=1548875&group_id=92314&atid=600306
* FIXME: rework this! just allow alphanumeric stuff or so?
* https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#naming_conventions
* http://superuser.com/questions/358855/what-characters-are-safe-in-cross-platform-file-names-for-linux-windows-and-os
* https://support.apple.com/en-us/HT202808
*/
if (OS.WINDOWS) {
plannedName = plannedName.replaceAll("\\?|\\*|\\<|\\>|\\||\\\"|\\:|\\.$|\\[|\\]", "");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract to a matcher?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out that this code is not needed since the FileUtil class already handles cleanup of file names

} else if (OS.OS_X) {
plannedName = plannedName.replace(":", "");
}

return plannedName;
}

public LinkedFile getFile() {
return linkedFile;
}
}
17 changes: 13 additions & 4 deletions src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
import org.jabref.preferences.JabRefPreferences;

import de.jensd.fx.glyphs.materialdesignicons.MaterialDesignIcon;
import de.jensd.fx.glyphs.materialdesignicons.utils.MaterialDesignIconFactory;
Expand All @@ -50,8 +51,8 @@ public class LinkedFilesEditor extends HBox implements FieldEditorFX {
@FXML private final LinkedFilesEditorViewModel viewModel;
@FXML private ListView<LinkedFileViewModel> listView;

public LinkedFilesEditor(String fieldName, DialogService dialogService, BibDatabaseContext databaseContext, TaskExecutor taskExecutor, AutoCompleteSuggestionProvider<?> suggestionProvider, FieldCheckers fieldCheckers) {
this.viewModel = new LinkedFilesEditorViewModel(fieldName, suggestionProvider, dialogService, databaseContext, taskExecutor, fieldCheckers);
public LinkedFilesEditor(String fieldName, DialogService dialogService, BibDatabaseContext databaseContext, TaskExecutor taskExecutor, AutoCompleteSuggestionProvider<?> suggestionProvider, FieldCheckers fieldCheckers, JabRefPreferences preferences) {
this.viewModel = new LinkedFilesEditorViewModel(fieldName, suggestionProvider, dialogService, databaseContext, taskExecutor, fieldCheckers, preferences);

ControlHelper.loadFXMLForControl(this);

Expand Down Expand Up @@ -150,8 +151,10 @@ private void handleOnDragDropped(LinkedFileViewModel originalItem, DragEvent eve
private static Node createFileDisplay(LinkedFileViewModel linkedFile) {
Text icon = MaterialDesignIconFactory.get().createIcon(linkedFile.getTypeIcon());
icon.setOnMouseClicked(event -> linkedFile.open());
Text link = new Text(linkedFile.getLink());
Text desc = new Text(linkedFile.getDescription());
Text link = new Text();
link.textProperty().bind(linkedFile.linkProperty());
Text desc = new Text();
desc.textProperty().bind(linkedFile.descriptionProperty());

ProgressBar progressIndicator = new ProgressBar();
progressIndicator.progressProperty().bind(linkedFile.downloadProgressProperty());
Expand Down Expand Up @@ -242,6 +245,9 @@ private ContextMenu createContextMenuForFile(LinkedFileViewModel linkedFile) {
MenuItem openFolder = new MenuItem(Localization.lang("Open folder"));
openFolder.setOnAction(event -> linkedFile.openFolder());

MenuItem download = new MenuItem(Localization.lang("Download file"));
download.setOnAction(event -> linkedFile.download());

MenuItem renameFile = new MenuItem(Localization.lang("Rename file"));
renameFile.setOnAction(event -> linkedFile.rename());
renameFile.setDisable(linkedFile.getFile().isOnlineLink());
Expand All @@ -261,6 +267,9 @@ private ContextMenu createContextMenuForFile(LinkedFileViewModel linkedFile) {
menu.getItems().add(new SeparatorMenuItem());
menu.getItems().addAll(openFile, openFolder);
menu.getItems().add(new SeparatorMenuItem());
if (linkedFile.getFile().isOnlineLink()) {
menu.getItems().add(download);
}
menu.getItems().addAll(renameFile, moveFile, deleteLink, deleteFile);

return menu;
Expand Down
Loading