on iOS, store only relative portion of directory path #277
Conversation
version (without NSDocumentDirectory) and converted to a full path only when the download is complete and the file needs to be copied This is consistent with Apple's suggestion and prevents errors where copy is attempted to a directory that no longer exists due to NSDocumentDirectory changing. Per documentation, one should not store a file path that includes NSDocumentDirectory, only the path following it, and reconstruct at runtime.
|
I'm very happy to see this being discussed. |
|
@781flyingdutchman, does this mean that currently, the DownloadTask returned by loadTasks() on ios might return an incorrect savedDir? |
|
Yes. Since iOS8 the NSDocumentDirectory changes for every app launch, so you should always rebuild that path and append the relative directory. For this reason, Apple recommends that you don't store the absolute path ever, only the relative path. In this PR I have changed the code to do that, but - as I explain above - it does mean this is a breaking change and it now behaves differently between iOS and Android. |
version (without NSDocumentDirectory) and converted to a full path only when the download is complete and the file needs to be copied This is consistent with Apple's suggestion and prevents errors where copy is attempted to a directory that no longer exists due to NSDocumentDirectory changing. Per documentation, one should not store a file path that includes NSDocumentDirectory, only the path following it, and reconstruct at runtime. Re-applied this fix to the null-safe version 1.6
|
I forked the repo & applied the changes you have made, for iOS, it does not work. |
On iOS the path represented by NSDocumentDirectory changes with every startup. Per Apple documentation, one should therefore never store the entire path, only the relative portion (that follows the NSDocumentDirectory).
In the current downloader code we accept the entire path, and go through some tricks to truncate it to the relative portion only (using [self shortenSavedDirPath:savedDir]). However, the full path is stored in the sqlite database, and this can lead to difficulties when that path is no longer valid by the time the application wakes up following a download.
This fork proposes to only every store the relative portion of the path in the sqlite database, to avoid problems with reconstructing or using it when the app restarts.
The pros of this change are: complies with Apple's advise and avoids errors when a download completes when the app has restarted (these are rare cases, but I use the plugin to download thousands of files and it does happen)
The con of this change is that the bahavior now is slightly different between iOS and Android: on Android we store the full path, whereas on iOS we store only the partial path.
One option to reconcile is to also only store the partial path on Android - I have not implemented that here, and in the app that uses this downloader I do a Platform.isiOS check to treat the savedDir field differently. That works, but it would have to be clearly documented.
@hnvn happy to discuss this a bit further as this is not as obvious a change