Skip to content

Commit

Permalink
Fix initial freeze when downloading files
Browse files Browse the repository at this point in the history
  • Loading branch information
tobiasdiez committed May 13, 2018
1 parent e5fa39a commit 468cce9
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import org.fxmisc.easybind.EasyBind;

public class FileDownloadTask extends BackgroundTask<Void> {
public class FileDownloadTask extends BackgroundTask<Path> {

private final URL source;
private final Path destination;
Expand All @@ -22,7 +22,7 @@ public FileDownloadTask(URL source, Path destination) {
}

@Override
protected Void call() throws Exception {
protected Path call() throws Exception {
URLDownload download = new URLDownload(source);
try (ProgressInputStream inputStream = download.asInputStream()) {
EasyBind.subscribe(
Expand All @@ -35,6 +35,6 @@ protected Void call() throws Exception {
Files.copy(inputStream, destination, StandardCopyOption.REPLACE_EXISTING);
}

return null;
return destination;
}
}
33 changes: 18 additions & 15 deletions src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -356,26 +356,30 @@ public void download() {
}

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));
URLDownload urlDownload = new URLDownload(linkedFile.getLink());
BackgroundTask<Path> downloadTask = BackgroundTask
.wrap(() -> {
Optional<ExternalFileType> suggestedType = inferFileType(urlDownload);
String suggestedTypeName = suggestedType.map(ExternalFileType::getName).orElse("");
linkedFile.setFileType(suggestedTypeName);

String suffix = suggestedType.map(ExternalFileType::getExtension).orElse("");
String suggestedName = getSuggestedFileName(suffix);
return targetDirectory.get().resolve(suggestedName);
})
.then(destination -> new FileDownloadTask(urlDownload.getSource(), destination))
.onSuccess(destination -> {
LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectoriesAsPaths(fileDirectoryPreferences));
linkedFile.setLink(newLinkedFile.getLink());
linkedFile.setFileType(newLinkedFile.getFileType());
})
.onFailure(exception -> dialogService.showErrorDialogAndWait("Download failed", exception));

downloadProgress.bind(downloadTask.workDonePercentageProperty());
taskExecutor.execute(downloadTask);
Expand All @@ -395,7 +399,6 @@ private Optional<ExternalFileType> inferFileType(URLDownload urlDownload) {
}

private Optional<ExternalFileType> inferFileTypeFromMimeType(URLDownload urlDownload) {
// TODO: what if this takes long time?
String mimeType = urlDownload.getMimeType();

if (mimeType != null) {
Expand Down
27 changes: 25 additions & 2 deletions src/main/java/org/jabref/gui/util/BackgroundTask.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.concurrent.Callable;
import java.util.concurrent.Future;
import java.util.function.Consumer;
import java.util.function.Function;

import javafx.beans.property.DoubleProperty;
import javafx.beans.property.ObjectProperty;
Expand Down Expand Up @@ -120,11 +121,33 @@ public BackgroundTask<V> onFinished(Runnable onFinished) {
return this;
}

/**
* Creates a {@link BackgroundTask} that first runs this task and based on the result runs a second task.
*
* @param nextTaskFactory the function that creates the new task
* @param <T> type of the return value of the second task
*/
public <T> BackgroundTask<T> then(Function<V, BackgroundTask<T>> nextTaskFactory) {
return new BackgroundTask<T>() {
@Override
protected T call() throws Exception {
V result = BackgroundTask.this.call();
BackgroundTask<T> nextTask = nextTaskFactory.apply(result);
EasyBind.subscribe(nextTask.progressProperty(), this::updateProgress);
return nextTask.call();
}
};
}

protected void updateProgress(BackgroundProgress newProgress) {
progress.setValue(newProgress);
}

protected void updateProgress(double workDone, double max) {
progress.setValue(new BackgroundProgress(workDone, max));
updateProgress(new BackgroundProgress(workDone, max));
}

public class BackgroundProgress {
class BackgroundProgress {

private final double workDone;
private final double max;
Expand Down

0 comments on commit 468cce9

Please sign in to comment.