-
Notifications
You must be signed in to change notification settings - Fork 60
Add support for periodically downloading puzzles in the background. #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
156 changes: 156 additions & 0 deletions
156
app/src/main/java/com/totsp/crossword/BackgroundDownloadService.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| package com.totsp.crossword; | ||
|
|
||
| import android.Manifest; | ||
| import android.annotation.TargetApi; | ||
| import android.app.NotificationManager; | ||
| import android.app.job.JobInfo; | ||
| import android.app.job.JobParameters; | ||
| import android.app.job.JobScheduler; | ||
| import android.app.job.JobService; | ||
| import android.content.ComponentName; | ||
| import android.content.Context; | ||
| import android.content.SharedPreferences; | ||
| import android.content.pm.PackageManager; | ||
| import android.os.AsyncTask; | ||
| import android.os.Looper; | ||
| import android.preference.PreferenceManager; | ||
| import android.support.v4.content.ContextCompat; | ||
|
|
||
| import com.totsp.crossword.net.Downloaders; | ||
|
|
||
| import java.util.Date; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
|
|
||
| // Currently only available on API version >=21 due to use of JobScheduler. | ||
| // It may be possible to implement this functionality using AlarmManager for lower SDK versions. | ||
| @TargetApi(21) | ||
| public class BackgroundDownloadService extends JobService { | ||
| public static final String DOWNLOAD_PENDING_PREFERENCE = "backgroundDlPending"; | ||
|
|
||
| private static final Logger LOGGER = | ||
| Logger.getLogger(BackgroundDownloadService.class.getCanonicalName()); | ||
|
|
||
| private static JobInfo getJobInfo(boolean requireUnmetered, boolean allowRoaming, | ||
| boolean requireCharging) { | ||
| JobInfo.Builder builder = new JobInfo.Builder( | ||
| JobSchedulerId.BACKGROUND_DOWNLOAD.id(), | ||
| new ComponentName("com.totsp.crossword.shortyz", | ||
| BackgroundDownloadService.class.getName())); | ||
|
|
||
| builder.setPeriodic(TimeUnit.HOURS.toMillis(1)) | ||
| .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED) | ||
| .setRequiresCharging(requireCharging) | ||
| .setPersisted(true); | ||
|
|
||
| if (!requireUnmetered) { | ||
| if (allowRoaming) { | ||
| builder.setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY); | ||
| } else { | ||
| builder.setRequiredNetworkType(JobInfo.NETWORK_TYPE_NOT_ROAMING); | ||
| } | ||
| } | ||
|
|
||
| return builder.build(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean onStartJob(JobParameters job) { | ||
| LOGGER.info("Starting background download task"); | ||
| DownloadTask downloadTask = new DownloadTask(this); | ||
| downloadTask.execute(job); | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean onStopJob(JobParameters job) { | ||
| return false; | ||
| } | ||
|
|
||
| public static void updateJob(Context context) { | ||
| SharedPreferences preferences = | ||
| PreferenceManager.getDefaultSharedPreferences(context); | ||
|
|
||
| boolean enable = preferences.getBoolean("backgroundDownload", false); | ||
|
|
||
| if (enable) { | ||
| scheduleJob(context); | ||
| } else { | ||
| cancelJob(context); | ||
| } | ||
| } | ||
|
|
||
| private static void scheduleJob(Context context) { | ||
| JobScheduler scheduler = | ||
| (JobScheduler)context.getSystemService(Context.JOB_SCHEDULER_SERVICE); | ||
|
|
||
| SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(context); | ||
|
|
||
| JobInfo info = getJobInfo( | ||
| preferences.getBoolean("backgroundDownloadRequireUnmetered", true), | ||
| preferences.getBoolean("backgroundDownloadAllowRoaming", false), | ||
| preferences.getBoolean("backgroundDownloadRequireCharging", false)); | ||
|
|
||
|
|
||
| LOGGER.info("Scheduling background download job: " + info); | ||
|
|
||
| int result = scheduler.schedule(info); | ||
|
|
||
| if (result == JobScheduler.RESULT_SUCCESS) { | ||
| LOGGER.info("Successfully scheduled background downloads"); | ||
| } else { | ||
| LOGGER.log(Level.WARNING, "Unable to schedule background downloads"); | ||
| } | ||
| } | ||
|
|
||
| private static void cancelJob(Context context) { | ||
| LOGGER.info("Unscheduling background downloads"); | ||
| JobScheduler scheduler = | ||
| (JobScheduler)context.getSystemService(Context.JOB_SCHEDULER_SERVICE); | ||
| scheduler.cancel(JobSchedulerId.BACKGROUND_DOWNLOAD.id()); | ||
| } | ||
|
|
||
| private static class DownloadTask extends AsyncTask<JobParameters, Void, JobParameters> { | ||
| private final JobService jobService; | ||
|
|
||
| public DownloadTask(JobService jobService) { | ||
| this.jobService = jobService; | ||
| } | ||
|
|
||
| @Override | ||
| protected JobParameters doInBackground(JobParameters... params) { | ||
| Context context = jobService.getApplicationContext(); | ||
|
|
||
| NotificationManager nm = | ||
| (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE); | ||
|
|
||
| if (ContextCompat.checkSelfPermission(context, | ||
| Manifest.permission.WRITE_EXTERNAL_STORAGE) != | ||
| PackageManager.PERMISSION_GRANTED) { | ||
| LOGGER.info("Skipping download, no write permission"); | ||
| return params[0]; | ||
| } | ||
|
|
||
| LOGGER.info("Downloading most recent puzzles"); | ||
|
|
||
| Looper.prepare(); | ||
|
|
||
| SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); | ||
| final Downloaders dls = new Downloaders(prefs, nm, context, false); | ||
| dls.downloadLatestIfNewerThanDate(new Date(), null); | ||
|
|
||
| // This is used to tell BrowseActivity that puzzles may have been updated while | ||
| // paused. | ||
| prefs.edit() | ||
| .putBoolean(DOWNLOAD_PENDING_PREFERENCE, true) | ||
| .apply(); | ||
|
|
||
| return params[0]; | ||
| } | ||
|
|
||
| protected void onPostExecute(JobParameters params) { | ||
| jobService.jobFinished(params, false); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package com.totsp.crossword; | ||
|
|
||
| // All JobScheduler Job IDs for this application. | ||
| // | ||
| // Using an enum here since all jobs scheduled by the same uid (not just package) need to be unique. | ||
| // | ||
| // These need to be stable across app updates. | ||
| public enum JobSchedulerId { | ||
| BACKGROUND_DOWNLOAD(10); | ||
|
|
||
| private int id; | ||
|
|
||
| JobSchedulerId(int id) { | ||
| this.id = id; | ||
| } | ||
|
|
||
| int id() { | ||
| return this.id; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would likely be better done at something like 07:00UTC. That generally means that the puzzles that update at Midnight pacific time will be there. Not a lot of reason to keep running all the DLs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did consider this, but the JobScheduler API doesn't support scheduling at a specific time of day (only periodically or after a given deadline) so scheduling the job consistently at 07:00UTC is non-trivial (the delay needs to change if the user changes time zones, DST, etc.). We'll also have to implement some handling of failed downloads, whereas now we just converge on the next hourly run.
If your main concern is device battery usage, I've been running this for the past few months and haven't seen Shortyz noticeable in my battery stats (obviously a pretty small sample size). My guess is JobScheduler batches pretty efficiently. There's also an option to download only while charging, which we could change to a default.
I also have a personal preference to make this work with PR #88 which would require also scheduling at 04:00UTC to get the NYT puzzle the night before. I could just hard code two daily runs to make this work for now if we go this route, so not a big deal.
Let me know how strongly you feel about this. I can spend some time figuring out a way to cut down the number of download attempts if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out that this is a bit more complicated than planned. It's possible to listen for timezone (and it seems DST) changes on the user's device via the TIME_CHANGED and TIMEZONE_CHANGED intents and reschedule your job/alarm accordingly.
However, we're actually trying to set an alarm in a different time zone (12AM Pacific as you mentioned). Since not everyone changes DST at the same time (or at all, e.g. Arizona), we need to reschedule the job when the PDT<->PST transition happens to keep the alarm at the right time of day.
If we just ditch periodic jobs and reschedule after a successful completion, it might work alright since the consequences of a miss by an hour or so in either direction aren't too bad (deliver late a couple times a year, or schedule an extra job). We'd have to make this work with handling transient download failures though.
That said, the Downloaders class will already prevent re-downloading if the puzzle exists. I just updated the PR with a fix for a bug around notifications here (a notification still got posted even if all downloads were skipped).
Given that making this work for users in all time zones is as bit complex, and we already avoid unnecessary network access, would you be alright leaving this as is for now?