781flyingdutchman / background_downloader

Flutter plugin for file downloads and uploads
Other
158 stars 73 forks source link

Scoped storage support on Android #14

Closed rekire closed 1 year ago

rekire commented 1 year ago

I am planning to add this support to my 2.0 fork, but why not also in your plugin. I'm just thinking if we could combine our resources. I wrote such code already in the past and for Android 14 I need to maintain it anyway.

I still need to evaluate your interface to understand better how you do it and what would be the best way to integrate that.

781flyingdutchman commented 1 year ago

Hi, what features from scoped storage on Android do you need? Are you just looking to download a file into the Downloads folder? Or the external files dir? Scoped storage is uniquely Android, so I wonder if the best way to implement this is as a separate "moveToScopedStorage" call that you call after the normal download completed, and that only works on Android. That way it doesn't clutter the API. The call would take the task (which includes already where the file was downloaded) and the Android specific destination (eg using an enum similar to the BaseDirectory enum). On native android you then check permissions, insert in Media store, then move the file.

What do you think? I'd be happy for you to create a pull request if you're interested in working together on this. Perhaps the final version can then eventually become Flutter Downloader V2.

rekire commented 1 year ago

For my usecase it would be fine to put the downloaded file into the Download folder, but I can also imagine cases where you need documents, photos, videos or music. Basically those directories exist on all platforms. I think that your BaseDirectory emum is a good way to go, just we might need a fallback e.g. desktop does not exists on mobile devices (even if there is a getter on iOS).

References for Android: https://developer.android.com/reference/android/provider/MediaStore References for Windows: https://learn.microsoft.com/en-us/windows/win32/shell/knownfolderid#constants References for MacOS/iOS: https://developer.apple.com/documentation/foundation/filemanager/searchpathdirectory

781flyingdutchman commented 1 year ago

I would still prefer to have it as a separate operation following a regular download. That keeps the core code clean and focused on the app-owned directories. To move to scoped storage (or equivalent on other platforms, to the extent possible) you then call moveToScopedStorage. I started a basic implementation in branch scoped_storage, where only the TODO in the AndroidSpecific.kt file needs to be done. Can you create a pull request with that branch as the base and work on this? If you already have an implementation for Android this should be quick. I am not familiar with the MediaStore.

rekire commented 1 year ago

I'll check what I can do. I'm fine that this would be a follow up step, but can we rename it to something like move to (well)knownFolder or something like that? Even if my initial request was scoped storage that topic should be relevant on other platforms too

781flyingdutchman commented 1 year ago

Sure, maybe MoveToPublicFolder? They key difference is that the BaseDirectory enum is private to the app and available on all platforms, so I don't want to mix in public folders that may or may not be available on all platforms. Thanks!

rekire commented 1 year ago

What about moveToKnownFolder from the multi user perspective of a Desktop that would be not necessary a public (shared) folder. Thx!

781flyingdutchman commented 1 year ago

It's public in that it's not private to the app: on all platforms you are moving the file out of the private scope and that is significant. I think KnownFolder is vague (every folder is known!), but if you don't like PublicFolder then perhaps SpecialFolder or ReservedFolder or MediaFolder? You need to include the right set of folders in the enum, and make sure there is a good equivalent of that for every platform if you think this can be expanded beyond Android. I am not sure you can simply move a file to the Videos folder on iOS, for example, but please try.

rekire commented 1 year ago

I understand what you mean and you got why I want to avoid public. SpecialFolder sounds very special, but a download or document folder is something normal. ReservedFolder sounds for me somehow exclusive... MediaFolder sounds like multimedia and would not match for desktop or downloads. What about PredefinedFolder, WellknownFolder or DefaultFolder?

rebaz94 commented 1 year ago

Hello there!

I wanted to let you know that I have implemented the moveToScopedStorage feature in my audio app. This implementation takes care of permissions (only applicable for Android version Q and above), adds the file to the Media store, and moves it to the specified destination. I have thoroughly tested this implementation and it works flawlessly.

You can check out the implementation in this commit.

If you're interested, I can create a pull request, or if you prefer, you could use this implementation as a starting point. Let me know what you think!

781flyingdutchman commented 1 year ago

Hi @rebaz94, thanks so much! This looks great. I have a few questions, comments and ideas, and it's easier to discuss those via a pull request, so I'd appreciate if you could create one. Are you open to some back-and-forth and to making further edits, or would you rather that I merge the pull request and make further modifications on my end? I'm ok either way, already very happy with your contribution!

781flyingdutchman commented 1 year ago

Hi @rebaz94, thanks so much! This looks great. I have a few questions, comments and ideas, and it's easier to discuss those via a pull request, so I'd appreciate if you could create one. Are you open to some back-and-forth and to making further edits, or would you rather that I merge the pull request and make further modifications on my end? I'm ok either way, already very happy with your contribution!

rebaz94 commented 1 year ago

@781flyingdutchman I'm glad you liked my contribution.

Unfortunately, I am currently working on multiple projects and don't have much time to spare for this particular one. However, I can create the initial pull request and you can make further modifications on your end as needed. Let me know if that works for you.

781flyingdutchman commented 1 year ago

That would be great, thanks! I won't get to this for a bit either (focused on notifications right now) but your contribution is very helpful in getting scoped storage implemented. Thanks again!

rebaz94 commented 1 year ago

I won't get to this for a bit either (focused on notifications right now)

oh yes, I really need this feature, so I will try do the required modification. here the PR #17

781flyingdutchman commented 1 year ago

Sorry, it's not implemented on the main branch - only on scoped_storage so I'd like to keep the issue open until it is published.

rekire commented 1 year ago

Oh sorry you are right

781flyingdutchman commented 1 year ago

@rebaz94 I've now implemented moveToSharedStorage which builds on your code for Android, and adds similar (but more restricted) functionality for iOS, MacOS, Linux and Windows, with a few small modifications to the API in your original scoped_storage branch. I've also added notifications, so you should now be able to use both.

rebaz94 commented 1 year ago

@781flyingdutchman Thank you for your work on implementing moveToSharedStorage! It's great to hear that you've added support for more platforms and notifications as well.

By the way, I noticed that in Android for older versions that don't support scoped storage, the app should move files to outside of the app. Currently, it looks like the code isn't doing anything for this case. I think it would be easy to add support for that

781flyingdutchman commented 1 year ago

I am not familiar with scoped storage. I've tested it on API 30 and it works there. Can you add the code for older Android versions? The plugin supports min API 24. Would appreciate it!

rebaz94 commented 1 year ago

Hi @781flyingdutchman Just implemented please check this commit

781flyingdutchman commented 1 year ago

Thanks. I added requesting permissions and published as version 5.4.2. Thanks for your help