Added DestinationPathName to DownloadFile

[tvandegraaf]

Ref #93

[SimonSimCity]

Thanks for the support here, but it only fixes half of the problem ... I'll need to take a look at how this can be fixed permanently also when using the temporary location ...

[tvandegraaf]

Maybe I didn't look that clear to the code and also without looking at the code now: but doesn't this also fill the new property with the location when it's a temporary one? This PR is actually not a solution for the referenced issue, since that's a bit more complicated.

Edit: No, you're right. The temporary location does not get set.

Edit2: I've looked in the implementation for Android. There is a way to get the local path with downloadManager.GetUriForDownloadedFile, as you did in your sample app; but it only gets set after the download has been completed. We don't know when the download has been completed and the DownloadManager has no events to hook into. I don't think it's appropriate to add a while loop (also because the question raises where in the code that should be). We can update the docs and decide that when using a temp location, you don't get the path in the new property.

It's not that great, but it's at least something.

The reason for this PR is actually because I'm showing a notification with action when the download has been completed. The action opens the file, but for that I need the have the file path where the file was located. Because the PathNameForDownloadedFile gets set initially and in a seperate method, I don't have a way to get that file path. I can invoke the call manually, however, but as I have some logic that renames the file whenever it already exists, I get an incorrect file path.

[SimonSimCity]

You're right in that the native Android download manager doesn't have a way to get notified if a property has changed. That's why I've implemented a timer to update the properties of all running downloads.

In this way we could just set the property after updating the status. https://github.com/SimonSimCity/Xamarin-CrossDownloadManager/blob/a5a28607d840be23dce4ed124e3907f8718c01d3/DownloadManager/Plugin.DownloadManager.Android/DownloadManagerImplementation.cs#L167-L171

On Android I have to set the path before the download starts, which is the reason why it's downloaded directly to the desired destination. If the download is finished, publish the destination you get from the native download manager.

On iOS there is a method which gets called when the download is finished and it also gives you the full destination path. Here's the excerpt of the code where I move the file to the desired destination after a successful download: https://github.com/SimonSimCity/Xamarin-CrossDownloadManager/blob/a5a28607d840be23dce4ed124e3907f8718c01d3/DownloadManager/Plugin.DownloadManager.iOS/UrlSessionDownloadDelegate.cs#L83-L93

Here you either have to publish destinationPathName or location if the destinationPathName was empty.

As it's very likely that UWP will stick around for quite a while, it'd be good if you could also find a way there. Maybe this excerpt from the samples already helps you getting on track: https://github.com/SimonSimCity/Xamarin-CrossDownloadManager/blob/6833954d14de1299277e5f6d0e65bfe3c63ee9b3/Sample/UWP/MainPage.xaml.cs#L81-L86

To be consistent, the variable on the IDownloadFile instance should be updated before the status is set to DownloadFileStatus.COMPLETED.

Please let me know if you disagree in any of the details I wrote. I let you know this because I won't have the time to test these changes ... Please test it and report back. I'll merge it when the changes are useful for all the cases.

[tvandegraaf]

Sounds good. Could you review the last commit I made?

Android When testing on a Samsung Tab S2, I've gotten content://downloads/all_downloads/{randomnumber} back, where {randomnumber} is actually a random number.

iOS When testing on the iPhone Simulator, I got the following: file:///Users/{USERNAME}/Library/Developer/CoreSimulator/Devices/{SOME_GUID}/data/Containers/Data/Application/{SOME_GUID}/Library/Caches/com.apple.nsurlsessiond/Downloads/{REVERSED_DOMAIN_NAME_OF_APP}/CFNetworkDownload_kyJT1R.tmp

UWP For UWP I took a different approach. At first I set it in ProgressChanged when the status was set to completed, but that was too late. So now I'm setting it in the StartDownloadAsync method, where you are setting the file.

I got the following back: C:\Workspace\git\Xamarin-CrossDownloadManager\Sample\UWP\bin\x86\Debug\AppX\10MB.zip

How about that?

[tvandegraaf]

I've added another commit which only sets the DestinationPathName with the DownloadManager.GetUriForDownloadedFile result when it is null, since that one is always giving a content:// result. Something I don't want, since I'm using a path I allocated using PathNameForDownloadedFile.

[tvandegraaf]

See last commit for the Code Review changes. Feel free to redo the review. Code quality first!

[SimonSimCity]

Looks very good, Thanks!

I'll have a look at what else I will take into this release and then push it out. Please bare with me if it takes a while since I'll be on holiday for 2 weeks from today on and this is a project purely developed in my spare-time 😊

[tvandegraaf]

Thank you.

I understand. For now I'll just use my own DLLs and switch to the new NuGet release whenever you are ready. Have a great holiday!