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 all commits
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
154 changes: 133 additions & 21 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,20 +27,28 @@
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;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.logic.cleanup.CleanupPreferences;
import org.jabref.logic.cleanup.MoveFilesCleanup;
import org.jabref.logic.cleanup.RenamePdfCleanup;
import org.jabref.logic.journals.JournalAbbreviationLoader;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.layout.LayoutFormatterPreferences;
import org.jabref.logic.net.URLDownload;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.logic.xmp.XmpPreferences;
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 All @@ -60,20 +70,35 @@ public class LinkedFileViewModel extends AbstractViewModel {
private final DialogService dialogService;
private final BibEntry entry;
private final TaskExecutor taskExecutor;
private final FileDirectoryPreferences fileDirectoryPreferences;
private final CleanupPreferences cleanupPreferences;
private final LayoutFormatterPreferences layoutFormatterPreferences;
private final XmpPreferences xmpPreferences;
private final String fileNamePattern;

/**
* @deprecated use {@link #LinkedFileViewModel(LinkedFile, BibEntry, BibDatabaseContext, TaskExecutor, DialogService, JabRefPreferences, JournalAbbreviationLoader)} instead
*/
@Deprecated
public LinkedFileViewModel(LinkedFile linkedFile, BibEntry entry, BibDatabaseContext databaseContext, TaskExecutor taskExecutor) {
this(linkedFile, entry, databaseContext, taskExecutor, new FXDialogService());
this(linkedFile, entry, databaseContext, taskExecutor, new FXDialogService(), Globals.prefs, Globals.journalAbbreviationLoader);
}

protected LinkedFileViewModel(LinkedFile linkedFile, BibEntry entry, BibDatabaseContext databaseContext,
TaskExecutor taskExecutor, DialogService dialogService) {
public LinkedFileViewModel(LinkedFile linkedFile, BibEntry entry, BibDatabaseContext databaseContext,
Copy link
Member

Choose a reason for hiding this comment

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

I think, only the IMPORT_FILENAMEPATTERN and preferences.getFileDirectoryPreferences() are needed. Why not pass them both (as quick fix regarding the information hiding). OK, a new preference object would even be better, but that costs more time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also the auto-link preferences as well as the import preferences are needed. You are definitely right, a custom preference object unifying them all would be nice. To be honest, I don't have the motivation right now to create one (especially since the focus of this PR is a different one).

TaskExecutor taskExecutor, DialogService dialogService, JabRefPreferences preferences, JournalAbbreviationLoader abbreviationLoader) {
this.linkedFile = linkedFile;
this.databaseContext = databaseContext;
this.entry = entry;
this.taskExecutor = taskExecutor;
this.dialogService = dialogService;

downloadOngoing.bind(downloadProgress.greaterThanOrEqualTo(0).and(downloadProgress.lessThan(100)));
cleanupPreferences = preferences.getCleanupPreferences(abbreviationLoader);
layoutFormatterPreferences = preferences.getLayoutFormatterPreferences(abbreviationLoader);
xmpPreferences = preferences.getXMPPreferences();
fileNamePattern = preferences.get(JabRefPreferences.IMPORT_FILENAMEPATTERN);
fileDirectoryPreferences = preferences.getFileDirectoryPreferences();

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

Expand All @@ -97,12 +122,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 @@ -154,7 +179,7 @@ public void openFolder() {
path = Paths.get(linkedFile.getLink());
} else {
// relative to file folder
for (Path folder : databaseContext.getFileDirectoriesAsPaths(Globals.prefs.getFileDirectoryPreferences())) {
for (Path folder : databaseContext.getFileDirectoriesAsPaths(fileDirectoryPreferences)) {
Path file = folder.resolve(linkedFile.getLink());
if (Files.exists(file)) {
path = file;
Expand All @@ -177,21 +202,21 @@ public void rename() {
// Cannot rename remote links
return;
}
Optional<Path> fileDir = databaseContext.getFirstExistingFileDir(Globals.prefs.getFileDirectoryPreferences());
Optional<Path> fileDir = databaseContext.getFirstExistingFileDir(fileDirectoryPreferences);
if (!fileDir.isPresent()) {
dialogService.showErrorDialogAndWait(
Localization.lang("Rename file"),
Localization.lang("File directory is not set or does not exist!"));
return;
}

Optional<Path> file = linkedFile.findIn(databaseContext, Globals.prefs.getFileDirectoryPreferences());
Optional<Path> file = linkedFile.findIn(databaseContext, fileDirectoryPreferences);
if ((file.isPresent()) && Files.exists(file.get())) {
RenamePdfCleanup pdfCleanup = new RenamePdfCleanup(false,
databaseContext,
Globals.prefs.getCleanupPreferences(Globals.journalAbbreviationLoader).getFileNamePattern(),
Globals.prefs.getLayoutFormatterPreferences(Globals.journalAbbreviationLoader),
Globals.prefs.getFileDirectoryPreferences(), linkedFile);
cleanupPreferences.getFileNamePattern(),
layoutFormatterPreferences,
Copy link
Member

Choose a reason for hiding this comment

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

Actually layoutFormatterPrefernces can be removed from all cleanup ops (did this the maintable beta), but out of scope for this pr

fileDirectoryPreferences, linkedFile);

String targetFileName = pdfCleanup.getTargetFileName(linkedFile, entry);

Expand Down Expand Up @@ -249,21 +274,21 @@ public void moveToDefaultDirectory() {
}

// Get target folder
Optional<Path> fileDir = databaseContext.getFirstExistingFileDir(Globals.prefs.getFileDirectoryPreferences());
Optional<Path> fileDir = databaseContext.getFirstExistingFileDir(fileDirectoryPreferences);
if (!fileDir.isPresent()) {
dialogService.showErrorDialogAndWait(
Localization.lang("Move file"),
Localization.lang("File directory is not set or does not exist!"));
return;
}

Optional<Path> file = linkedFile.findIn(databaseContext, Globals.prefs.getFileDirectoryPreferences());
Optional<Path> file = linkedFile.findIn(databaseContext, fileDirectoryPreferences);
if ((file.isPresent()) && Files.exists(file.get())) {
// Linked file exists, so move it
MoveFilesCleanup moveFiles = new MoveFilesCleanup(databaseContext,
Globals.prefs.getCleanupPreferences(Globals.journalAbbreviationLoader).getFileDirPattern(),
Globals.prefs.getFileDirectoryPreferences(),
Globals.prefs.getLayoutFormatterPreferences(Globals.journalAbbreviationLoader), linkedFile);
cleanupPreferences.getFileDirPattern(),
fileDirectoryPreferences,
layoutFormatterPreferences, linkedFile);

boolean confirm = dialogService.showConfirmationDialogAndWait(
Localization.lang("Move file"),
Expand Down Expand Up @@ -325,13 +350,13 @@ public void edit() {
public void writeXMPMetadata() {
// Localization.lang("Writing XMP-metadata...")
BackgroundTask<Void> writeTask = BackgroundTask.wrap(() -> {
Optional<Path> file = linkedFile.findIn(databaseContext, Globals.prefs.getFileDirectoryPreferences());
Optional<Path> file = linkedFile.findIn(databaseContext, fileDirectoryPreferences);
if (!file.isPresent()) {
// TODO: Print error message
// Localization.lang("PDF does not exist");
} else {
try {
XmpUtilWriter.writeXmp(file.get(), entry, databaseContext.getDatabase(), Globals.prefs.getXMPPreferences());
XmpUtilWriter.writeXmp(file.get(), entry, databaseContext.getDatabase(), xmpPreferences);
} catch (IOException | TransformerException ex) {
// TODO: Print error message
// Localization.lang("Error while writing") + " '" + file.toString() + "': " + ex;
Expand All @@ -345,4 +370,91 @@ 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);

Optional<Path> targetDirectory = databaseContext.getFirstExistingFileDir(fileDirectoryPreferences);
if (!targetDirectory.isPresent()) {
dialogService.showErrorDialogAndWait(
Localization.lang("Download file"),
Localization.lang("File directory is not set or does not exist!"));
return;
}
String suffix = suggestedType.map(ExternalFileType::getExtension).orElse("");
String suggestedName = getSuggestedFileName(suffix);
Path destination = targetDirectory.get().resolve(suggestedName);

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

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

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, fileNamePattern);

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

public LinkedFile getFile() {
return linkedFile;
}
}
Loading