fluttercommunity / flutter_downloader

Flutter Downloader - A plugin for creating and managing download tasks.
https://pub.dev/packages/flutter_downloader
BSD 3-Clause "New" or "Revised" License
908 stars 511 forks source link

Clean up api #256

Open MarcelGarus opened 4 years ago

MarcelGarus commented 4 years ago

While the flutter_downloader works fine, its API doesn't conform to idiomatic Dart standards in some cases. That's why I'm currently rewriting the Dart side of the plugin to make it more intuitive. If implemented, this will be a breaking change and thus require a new major version update.

Especially the DownloadTasks are currently merely a wrapper around some data and not really actionable. I'm looking forward to rewriting the API so that something like the following is possible:

final task = await DownloadTask.create(
  url: 'https://...',
  downloadDirectory: getApplicationDirectory(),
);
task.updates.forEach(print);
task.onCompleted(() => task.openFile());

await Future.delayed(Duration(seconds: 3));
if (!task.isCompleted) {
  print('Download takes longer than three seconds.');
}

await task.waitUntilCompleted();

By using Streams rather than callbacks, it's possible to leverage Dart's strengths of async/await, applying stream transformations and support from the ecosystem. For example, one could easily build a Flutter widget like the following:

// inside a widget's State

DownloadTask task;

void _startDownload() {
  setState(() => task = DownloadTask.create(…));
}

Widget build(BuildContext context) {
  return Column(
    children: <Widget>[
      RaisedButton(onPressed: _startDownload, child: Text('Start download')),
      StreamBuilder<void>(
        stream: task?.updates ?? Stream.empty(),
        builder: (context, _) => Text('Progress: ${task?.progress}'),
      ),
    ],
  );
}

What do you think about this API?

hnvn commented 4 years ago

Sounds great. Let me know if you need supports in native sides

MarcelGarus commented 4 years ago

When invoking the native method enqueue with the arguments {url: https://placekitten.com/263/300, saved_dir: /data/user/0/vn.hunghd.example/app_flutter, file_name: null, headers: , show_notification: true, open_file_from_notification: true, requires_storage_not_low: true}, I'm getting the following error:

I/flutter (18668): {url: https://placekitten.com/263/300, saved_dir: /data/user/0/vn.hunghd.example/app_flutter, file_name: null, headers: , show_notification: true, open_file_from_notification: true, requires_storage_not_low: true}
W/WM-WorkSpec(18668): Backoff delay duration less than minimum value
D/DownloadWorker(18668): DownloadWorker{url=https://placekitten.com/263/300,filename=null,savedDir=/data/user/0/vn.hunghd.example/app_flutter,header=,isResume=false
D/skia    (18668): --- Failed to create image decoder with message 'unimplemented'
D/DownloadWorker(18668): Open connection to https://placekitten.com/263/300
D/DownloadWorker(18668): Content-Type = image/jpeg
D/DownloadWorker(18668): Content-Length = -1
D/DownloadWorker(18668): Charset = null
D/DownloadWorker(18668): Content-Disposition = null
D/DownloadWorker(18668): fileName = 300
D/skia    (18668): --- Failed to create image decoder with message 'unimplemented'
D/skia    (18668): --- Failed to create image decoder with message 'unimplemented'
W/System.err(18668): java.lang.IllegalArgumentException: Failed to find configured root that contains /data/data/vn.hunghd.example/app_flutter/300
W/System.err(18668):    at androidx.core.content.FileProvider$SimplePathStrategy.getUriForFile(FileProvider.java:744)
W/System.err(18668):    at androidx.core.content.FileProvider.getUriForFile(FileProvider.java:418)
W/System.err(18668):    at vn.hunghd.flutterdownloader.IntentUtils.buildIntent(IntentUtils.java:23)
W/System.err(18668):    at vn.hunghd.flutterdownloader.IntentUtils.validatedFileIntent(IntentUtils.java:35)
W/System.err(18668):    at vn.hunghd.flutterdownloader.DownloadWorker.downloadFile(DownloadWorker.java:361)
W/System.err(18668):    at vn.hunghd.flutterdownloader.DownloadWorker.doWork(DownloadWorker.java:189)
W/System.err(18668):    at androidx.work.Worker$1.run(Worker.java:85)
W/System.err(18668):    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
W/System.err(18668):    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
W/System.err(18668):    at java.lang.Thread.run(Thread.java:919)
I/WM-WorkerWrapper(18668): Worker result FAILURE for Work [ id=b24c29d5-139c-45d8-a8fd-e4c00fa6512e, tags={ flutter_download_task, vn.hunghd.flutterdownloader.DownloadWorker } ]

Any idea why that occurs?

MarcelGarus commented 4 years ago

I temporarily fixed this error with the help of @JonasWanke. The issue was that on Android, the package only supports saving to external directories, not to app directories. That's probably because the resolution of the DownloadedFileProvider fails. I'm no expert on native Android though, so somebody else should have a look at that. For now, I use the same workaround as the example and do Platform.isAndroid ? await getExternalStorageDirectory() : await getApplicationDocumentsDirectory().

Regarding some architectural decisions that I stumbled upon: Why was Java used instead of Kotlin? Why is there an SQL database in the background instead of just communicating with the Dart side?
And finally, I didn't change anything on the native side, but it gives me strange progress values like -101300. What's up with that?

hnvn commented 4 years ago

Why was Java used instead of Kotlin?

I prefer Java to Kotlin because I don't have much experiences in Kotlin.

Why is there an SQL database in the background instead of just communicating with the Dart side?

If there's no SQL database, where do you store download task data? If you ask me that why the plugin need to maintain the download task data, my answer is that it's designed in that way, a library for downloading management that users can fire and forget instead of just creating a download task and leaving users with all stuff of download state management.

hnvn commented 4 years ago

BTW, your issue related to Android Storage problem, it's not true that the plugin only supports saving to external directories. You can save your files in internal storage if you set the value of open_file_from_notification to false. It deals to the files management in Android. In order to open a file and preview it, the plugin needs to pass it to other applications on your device that can handle that file type (You can image it like the way you open files in your Windows computer, you want to open a FLV file, you need an application like VLC on your machine) and other applications can only access these files if they are stored in external storage.

MarcelGarus commented 4 years ago

Ohhh I see. I'm not experienced with native Android and didn't know that limitation exists, but it makes sense. We should probably document that behavior in the API as well.

MarcelGarus commented 4 years ago

In the related issue in our app (https://github.com/schul-cloud/schulcloud-flutter/issues/61), @JonasWanke mentioned that it's possible to store downloaded files in your app's private directory and grant other apps temporary read access to it: https://developer.android.com/training/secure-file-sharing/share-file#GrantPermissions This is also implemented in the open_file package: https://github.com/crazecoder/open_file/blob/34dc2cbdeb0d49a83be59f66cbc6e1a258dc2420/android/src/main/java/com/crazecoder/openfile/OpenFilePlugin.java#L142

hnvn commented 4 years ago

The plugin has already implemented that flow. Have a look at: https://github.com/fluttercommunity/flutter_downloader/blob/master/android/src/main/java/vn/hunghd/flutterdownloader/IntentUtils.java#L29

nioncode commented 4 years ago

@marcelgarus Will this also take care of moving the callback from the background isolate of the native plugin to the main isolate in the UI so that we don't have to manage this ourselves?

MarcelGarus commented 4 years ago

Of course! In Dart, most of the time, asynchronous operations are the way to go. So, on any DownloadTask, you'll be able to just call task.updates to receive a Stream of updates, which you can listen to or just plug into a StreamBuilder. Also, the DownloadTasks update their fields automatically so you can do:

var task = ...;
await Future.delayed(Duration(seconds: 1));
print(task.status);
sqrg commented 4 years ago

Any updates on this? Is there any workaround to wait for a file to finish downloading?

MarcelGarus commented 4 years ago

There are some minor issues left in the PR https://github.com/fluttercommunity/flutter_downloader/pull/283. Especially if you have macOS tooling and have some spare time, feel very welcome to look at the code from the PR.

In the meantime, you could register a callback for download progress, create a Completer and complete it as soon as you receive the callback for the task being finished.

sqrg commented 4 years ago

Sure, I can take a look on macOS. I don't have much experience nor knowledge, but I'll take a look

qq326646683 commented 4 years ago

same issue on Android, these works for me:

provider_paths.xml:

<?xml version="1.0" encoding="utf-8"?>
<paths xmlns:android="http://schemas.android.com/apk/res/android">
    <root-path
        name="root"
        path="." />
</paths>
arcanebrownie commented 3 years ago

I'm new to flutter, so I apologize if my question is stupid, but how do I use the DownloadTask.create that you show in here? Is the import still import 'package:flutter_downloader/flutter_downloader.dart';? Since that is the import I saw you use in the example.

Thanks!

bartekpacia commented 2 years ago

It's a pity that this PR died, would be great if the API had better design.