Add option to download only via wifi #500
Conversation
lyio
force-pushed
the
feature/408-wifi-only-downloads
branch
from
f19e9ab to
7965518
Compare
lyio
force-pushed
the
feature/408-wifi-only-downloads
branch
2 times, most recently
from
94b2649 to
8ea7dee
Compare
|
@hnvn Is this something you would consider reviewing if the conflicts are resolved? |
kuhnroyal
force-pushed
the
feature/408-wifi-only-downloads
branch
2 times, most recently
from
4973a4c to
ce0e79e
Compare
|
Thanks @lyio and @kuhnroyal for this PR. Android implementation is good for me but it seems that iOS implementation is missing configuration for DB scheme. On iOS, we have a pre-defined DB scheme in file |
|
@hnvn it works fine on iOS without updating the schema for some reason. I'll see to it that the file is updated, though. |
lyio
force-pushed
the
feature/408-wifi-only-downloads
branch
from
ce0e79e to
a0ec2c5
Compare
|
@hnvn I have a couple of questions regarding the scheme. I have updated the binary database file to include a new column I have not seen anything in the Objective-C code suggesting that any form of migration strategy is applied. On Android the database is cleared and recreated when changes are made, but on iOS I don't see anything like that. Am I missing something here? Do we need to add this first? Because otherwise my changes will have no effect on existing installations as the file is only copied over if it does not exist yet. |
|
The iOS implementation comes without migration solution. I currently don't find down any solution for it. That's why I am quit hesitate to add new field to DB schema. New field on DB need to release as a major update |
|
The Android solution to the problem seems to be, to simply delete the DB and create it again when the version changes. |
bartekpacia
changed the title
|
@hnvn do you have a migration path for iOS in mind? |
+ Add `allowCellular` flag to the `enqueue` that defaults to `true` + Add separate `NSURLSession` on iOS that sets `allowsCellularAccess` false in its configuration + Schedule download requests with one of the two sessions depending on the flag of the download + Add the flag to the Android plugin and set the network constraints to either `NetworkType.CONNECTED` or `NetworkType.UNMETERED` depending on the value of the flag + Migrate sql database on Android to keep track of the flag
+ Add `allow_cellular` column to the prepared database schema
lyio
force-pushed
the
feature/408-wifi-only-downloads
branch
from
a0ec2c5 to
f095790
Compare
|
|
||
| NSURLSessionDownloadTask *task = [self downloadTaskWithURL:[NSURL URLWithString:urlString] fileName:fileName andSavedDir:savedDir andHeaders:headers]; | ||
| NSURLSessionDownloadTask *task = [self downloadTaskWithURL:[NSURL URLWithString:urlString] fileName:fileName andSavedDir:savedDir andHeaders:headers allowCellular:allowCellular]; |
There was a problem hiding this comment.
After integrating the changes to how the sessions are initialized (in a if(_isolate) condition), this always returns nil. The plugin is initialized twice, once in the foreground where no sessions are configured and once in the background where there are.
The enqueue call is made on the foreground instance though, where the sessions are missing.
I'm having trouble understanding why this is working in the example and on the master branch, @hnvn.
There was a problem hiding this comment.
Disregard this, I have found the problem, I think
lyio
force-pushed
the
feature/408-wifi-only-downloads
branch
from
f095790 to
b53fac0
Compare
|
Due to all the changes in the meantime (especially the migration to Kotlin), I have a hard time rebasing this and still getting it to work. I will close this PR and make a new one. |
allowCellularflag to theenqueuethat defaults totrueNSURLSessionon iOS thatsets
allowsCellularAccessfalse in its configurationtwo sessions depending on the flag of the download
network constraints to either
NetworkType.CONNECTEDorNetworkType.UNMETEREDdepending on the value of the flagthe flag
Closes #408