From 640412314c7756c99e9ad4d9066d1a2d6910fca3 Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Wed, 29 Jul 2020 14:30:44 -0500 Subject: [PATCH] Throttle WorkManager calls to 200/min --- app/src/main/java/org/tasks/db/DbUtils.kt | 5 - .../main/java/org/tasks/db/SuspendDbUtils.kt | 6 +- .../main/java/org/tasks/jobs/BackupWork.kt | 2 +- .../org/tasks/jobs/MidnightRefreshWork.kt | 2 +- .../main/java/org/tasks/jobs/RefreshWork.kt | 2 +- .../java/org/tasks/jobs/RepeatingWorker.kt | 2 +- .../main/java/org/tasks/jobs/WorkManager.kt | 20 +-- .../java/org/tasks/jobs/WorkManagerImpl.kt | 131 ++++++++++-------- .../preferences/fragments/Synchronization.kt | 12 +- .../org/tasks/scheduling/RefreshScheduler.kt | 6 +- 10 files changed, 104 insertions(+), 84 deletions(-) diff --git a/app/src/main/java/org/tasks/db/DbUtils.kt b/app/src/main/java/org/tasks/db/DbUtils.kt index 79304b536..fdbe429da 100644 --- a/app/src/main/java/org/tasks/db/DbUtils.kt +++ b/app/src/main/java/org/tasks/db/DbUtils.kt @@ -4,9 +4,4 @@ object DbUtils { const val MAX_SQLITE_ARGS = 990 fun Iterable.dbchunk(): List> = chunked(MAX_SQLITE_ARGS) - - fun Iterable.eachChunk(action: (List) -> Unit) = dbchunk().forEach(action) - - fun Iterable.chunkedMap(transform: (List) -> Iterable): List = - dbchunk().flatMap(transform) } \ No newline at end of file diff --git a/app/src/main/java/org/tasks/db/SuspendDbUtils.kt b/app/src/main/java/org/tasks/db/SuspendDbUtils.kt index 4119fb4a6..1f21203bc 100644 --- a/app/src/main/java/org/tasks/db/SuspendDbUtils.kt +++ b/app/src/main/java/org/tasks/db/SuspendDbUtils.kt @@ -1,10 +1,14 @@ package org.tasks.db +import org.tasks.db.DbUtils.MAX_SQLITE_ARGS import org.tasks.db.DbUtils.dbchunk object SuspendDbUtils { suspend fun Iterable.eachChunk(action: suspend (List) -> Unit) = - dbchunk().forEach { action.invoke(it) } + eachChunk(MAX_SQLITE_ARGS, action) + + suspend fun Iterable.eachChunk(size: Int, action: suspend (List) -> Unit) = + chunked(size).forEach { action.invoke(it) } suspend fun Iterable.chunkedMap(transform: suspend (List) -> Iterable): List = dbchunk().flatMap { transform.invoke(it) } diff --git a/app/src/main/java/org/tasks/jobs/BackupWork.kt b/app/src/main/java/org/tasks/jobs/BackupWork.kt index 38d177833..675ff3447 100644 --- a/app/src/main/java/org/tasks/jobs/BackupWork.kt +++ b/app/src/main/java/org/tasks/jobs/BackupWork.kt @@ -30,7 +30,7 @@ class BackupWork @WorkerInject constructor( return Result.success() } - override fun scheduleNext() = workManager.scheduleBackup() + override suspend fun scheduleNext() = workManager.scheduleBackup() private suspend fun startBackup(context: Context?) { try { diff --git a/app/src/main/java/org/tasks/jobs/MidnightRefreshWork.kt b/app/src/main/java/org/tasks/jobs/MidnightRefreshWork.kt index 342a2263f..326744c7a 100644 --- a/app/src/main/java/org/tasks/jobs/MidnightRefreshWork.kt +++ b/app/src/main/java/org/tasks/jobs/MidnightRefreshWork.kt @@ -19,5 +19,5 @@ class MidnightRefreshWork @WorkerInject constructor( return Result.success() } - override fun scheduleNext() = workManager.scheduleMidnightRefresh() + override suspend fun scheduleNext() = workManager.scheduleMidnightRefresh() } \ No newline at end of file diff --git a/app/src/main/java/org/tasks/jobs/RefreshWork.kt b/app/src/main/java/org/tasks/jobs/RefreshWork.kt index 061bd45d5..69dacf9a2 100644 --- a/app/src/main/java/org/tasks/jobs/RefreshWork.kt +++ b/app/src/main/java/org/tasks/jobs/RefreshWork.kt @@ -20,5 +20,5 @@ class RefreshWork @WorkerInject constructor( return Result.success() } - override fun scheduleNext() = refreshScheduler.scheduleNext() + override suspend fun scheduleNext() = refreshScheduler.scheduleNext() } \ No newline at end of file diff --git a/app/src/main/java/org/tasks/jobs/RepeatingWorker.kt b/app/src/main/java/org/tasks/jobs/RepeatingWorker.kt index 493c5a363..9586a4df8 100644 --- a/app/src/main/java/org/tasks/jobs/RepeatingWorker.kt +++ b/app/src/main/java/org/tasks/jobs/RepeatingWorker.kt @@ -16,5 +16,5 @@ abstract class RepeatingWorker internal constructor( return result } - protected abstract fun scheduleNext() + protected abstract suspend fun scheduleNext() } \ No newline at end of file diff --git a/app/src/main/java/org/tasks/jobs/WorkManager.kt b/app/src/main/java/org/tasks/jobs/WorkManager.kt index c942fca6f..adc91ba93 100644 --- a/app/src/main/java/org/tasks/jobs/WorkManager.kt +++ b/app/src/main/java/org/tasks/jobs/WorkManager.kt @@ -7,32 +7,32 @@ import org.tasks.data.Place interface WorkManager { - fun afterComplete(current: Task, original: Task?) + suspend fun afterComplete(task: Task) - fun cleanup(ids: Iterable) + suspend fun cleanup(ids: Iterable) - fun sync(immediate: Boolean) + suspend fun sync(immediate: Boolean) - fun reverseGeocode(place: Place) + suspend fun reverseGeocode(place: Place) - fun updateBackgroundSync() + suspend fun updateBackgroundSync() suspend fun updateBackgroundSync( forceAccountPresent: Boolean?, forceBackgroundEnabled: Boolean?, forceOnlyOnUnmetered: Boolean?) - fun scheduleRefresh(time: Long) + suspend fun scheduleRefresh(time: Long) - fun scheduleMidnightRefresh() + suspend fun scheduleMidnightRefresh() fun scheduleNotification(scheduledTime: Long) - fun scheduleBackup() + suspend fun scheduleBackup() - fun scheduleConfigRefresh() + suspend fun scheduleConfigRefresh() - fun scheduleDriveUpload(uri: Uri, purge: Boolean) + suspend fun scheduleDriveUpload(uri: Uri, purge: Boolean) fun cancelNotifications() diff --git a/app/src/main/java/org/tasks/jobs/WorkManagerImpl.kt b/app/src/main/java/org/tasks/jobs/WorkManagerImpl.kt index 2d7a192cb..5c3f8e3cc 100644 --- a/app/src/main/java/org/tasks/jobs/WorkManagerImpl.kt +++ b/app/src/main/java/org/tasks/jobs/WorkManagerImpl.kt @@ -15,9 +15,9 @@ import org.tasks.R import org.tasks.data.CaldavDao import org.tasks.data.GoogleTaskListDao import org.tasks.data.Place -import org.tasks.data.runBlocking import org.tasks.date.DateTimeUtils.midnight import org.tasks.date.DateTimeUtils.newDateTime +import org.tasks.db.SuspendDbUtils.eachChunk import org.tasks.jobs.WorkManager.Companion.MAX_CLEANUP_LENGTH import org.tasks.jobs.WorkManager.Companion.REMOTE_CONFIG_INTERVAL_HOURS import org.tasks.jobs.WorkManager.Companion.TAG_BACKGROUND_SYNC @@ -26,6 +26,7 @@ import org.tasks.jobs.WorkManager.Companion.TAG_MIDNIGHT_REFRESH import org.tasks.jobs.WorkManager.Companion.TAG_REFRESH import org.tasks.jobs.WorkManager.Companion.TAG_REMOTE_CONFIG import org.tasks.jobs.WorkManager.Companion.TAG_SYNC +import org.tasks.notifications.Throttle import org.tasks.preferences.Preferences import org.tasks.time.DateTimeUtils import timber.log.Timber @@ -38,32 +39,42 @@ class WorkManagerImpl constructor( private val preferences: Preferences, private val googleTaskListDao: GoogleTaskListDao, private val caldavDao: CaldavDao): WorkManager { - + private val throttle = Throttle(200, 60000, "WORK") private val alarmManager: AlarmManager = context.getSystemService(Context.ALARM_SERVICE) as AlarmManager private val workManager = androidx.work.WorkManager.getInstance(context) - override fun afterComplete(current: Task, original: Task?) { - workManager.enqueue( + private suspend fun enqueue(builder: WorkRequest.Builder<*, *>) { + throttle.run { + workManager.enqueue(builder.build()) + } + } + + private suspend fun enqueue(continuation: WorkContinuation) { + throttle.run { + continuation.enqueue() + } + } + + override suspend fun afterComplete(task: Task) { + enqueue( OneTimeWorkRequest.Builder(AfterSaveWork::class.java) .setInputData(Data.Builder() - .putLong(AfterSaveWork.EXTRA_ID, current.id) - .build()) - .build()) + .putLong(AfterSaveWork.EXTRA_ID, task.id) + .build())) } - override fun cleanup(ids: Iterable) { - ids.chunked(MAX_CLEANUP_LENGTH) { - workManager.enqueue( + override suspend fun cleanup(ids: Iterable) { + ids.eachChunk(MAX_CLEANUP_LENGTH) { + enqueue( OneTimeWorkRequest.Builder(CleanupWork::class.java) .setInputData( Data.Builder() .putLongArray(CleanupWork.EXTRA_TASK_IDS, it.toLongArray()) - .build()) - .build()) + .build())) } } - override fun sync(immediate: Boolean) { + override suspend fun sync(immediate: Boolean) { val constraints = Constraints.Builder() .setRequiredNetworkType( if (!immediate && preferences.getBoolean(R.string.p_background_sync_unmetered_only, false)) { @@ -78,24 +89,26 @@ class WorkManagerImpl constructor( if (!immediate) { builder.setInitialDelay(1, TimeUnit.MINUTES) } - val request = builder.build() - workManager.beginUniqueWork(TAG_SYNC, ExistingWorkPolicy.REPLACE, request).enqueue() + throttle.run { + workManager + .beginUniqueWork(TAG_SYNC, ExistingWorkPolicy.REPLACE, builder.build()) + .enqueue() + } } - override fun reverseGeocode(place: Place) { + override suspend fun reverseGeocode(place: Place) { if (BuildConfig.DEBUG && place.id == 0L) { throw RuntimeException("Missing id") } - workManager.enqueue( + enqueue( OneTimeWorkRequest.Builder(ReverseGeocodeWork::class.java) .setInputData(Data.Builder().putLong(ReverseGeocodeWork.PLACE_ID, place.id).build()) .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 1, TimeUnit.MINUTES) .setConstraints( - Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build()) - .build()) + Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build())) } - override fun updateBackgroundSync() = runBlocking { + override suspend fun updateBackgroundSync() { updateBackgroundSync(null, null, null) } @@ -114,24 +127,26 @@ class WorkManagerImpl constructor( scheduleBackgroundSync(backgroundEnabled && accountsPresent, onlyOnWifi) } - private fun scheduleBackgroundSync(enabled: Boolean, onlyOnUnmetered: Boolean) { + private suspend fun scheduleBackgroundSync(enabled: Boolean, onlyOnUnmetered: Boolean) { Timber.d("background sync enabled: %s, onlyOnUnmetered: %s", enabled, onlyOnUnmetered) - if (enabled) { - workManager.enqueueUniquePeriodicWork( - TAG_BACKGROUND_SYNC, - ExistingPeriodicWorkPolicy.KEEP, - PeriodicWorkRequest.Builder(SyncWork::class.java, 1, TimeUnit.HOURS) - .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 1, TimeUnit.MINUTES) - .setConstraints(getNetworkConstraints(onlyOnUnmetered)) - .build()) - } else { - workManager.cancelUniqueWork(TAG_BACKGROUND_SYNC) + throttle.run { + if (enabled) { + workManager.enqueueUniquePeriodicWork( + TAG_BACKGROUND_SYNC, + ExistingPeriodicWorkPolicy.KEEP, + PeriodicWorkRequest.Builder(SyncWork::class.java, 1, TimeUnit.HOURS) + .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 1, TimeUnit.MINUTES) + .setConstraints(getNetworkConstraints(onlyOnUnmetered)) + .build()) + } else { + workManager.cancelUniqueWork(TAG_BACKGROUND_SYNC) + } } } - override fun scheduleRefresh(time: Long) = enqueueUnique(TAG_REFRESH, RefreshWork::class.java, time) + override suspend fun scheduleRefresh(time: Long) = enqueueUnique(TAG_REFRESH, RefreshWork::class.java, time) - override fun scheduleMidnightRefresh() = + override suspend fun scheduleMidnightRefresh() = enqueueUnique(TAG_MIDNIGHT_REFRESH, MidnightRefreshWork::class.java, midnight()) override fun scheduleNotification(scheduledTime: Long) { @@ -149,29 +164,30 @@ class WorkManagerImpl constructor( } } - override fun scheduleBackup() { - enqueueUnique( - TAG_BACKUP, - BackupWork::class.java, - newDateTime(preferences.getLong(R.string.p_last_backup, 0L)) - .plusDays(1) - .millis - .coerceAtMost(midnight())) - } - - override fun scheduleConfigRefresh() { - workManager.enqueueUniquePeriodicWork( - TAG_REMOTE_CONFIG, - ExistingPeriodicWorkPolicy.KEEP, - PeriodicWorkRequest.Builder( - RemoteConfigWork::class.java, REMOTE_CONFIG_INTERVAL_HOURS, TimeUnit.HOURS) - .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 1, TimeUnit.MINUTES) - .setConstraints( - Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build()) - .build()) + override suspend fun scheduleBackup() = + enqueueUnique( + TAG_BACKUP, + BackupWork::class.java, + newDateTime(preferences.getLong(R.string.p_last_backup, 0L)) + .plusDays(1) + .millis + .coerceAtMost(midnight())) + + override suspend fun scheduleConfigRefresh() { + throttle.run { + workManager.enqueueUniquePeriodicWork( + TAG_REMOTE_CONFIG, + ExistingPeriodicWorkPolicy.KEEP, + PeriodicWorkRequest.Builder( + RemoteConfigWork::class.java, REMOTE_CONFIG_INTERVAL_HOURS, TimeUnit.HOURS) + .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 1, TimeUnit.MINUTES) + .setConstraints( + Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build()) + .build()) + } } - override fun scheduleDriveUpload(uri: Uri, purge: Boolean) { + override suspend fun scheduleDriveUpload(uri: Uri, purge: Boolean) { if (!preferences.getBoolean(R.string.p_google_drive_backup, false)) { return } @@ -181,7 +197,7 @@ class WorkManagerImpl constructor( if (purge) { builder.setInitialDelay(Random().nextInt(3600).toLong(), TimeUnit.SECONDS) } - workManager.enqueue(builder.build()) + enqueue(builder) } private val networkConstraints: Constraints @@ -193,14 +209,15 @@ class WorkManagerImpl constructor( .setRequiredNetworkType(if (unmeteredOnly) NetworkType.UNMETERED else NetworkType.CONNECTED) .build() - private fun enqueueUnique(key: String, c: Class, time: Long) { + @SuppressLint("EnqueueWork") + private suspend fun enqueueUnique(key: String, c: Class, time: Long) { val delay = time - DateUtilities.now() val builder = OneTimeWorkRequest.Builder(c) if (delay > 0) { builder.setInitialDelay(delay, TimeUnit.MILLISECONDS) } Timber.d("$key: ${DateTimeUtils.printTimestamp(time)} (${DateTimeUtils.printDuration(delay)})") - workManager.beginUniqueWork(key, ExistingWorkPolicy.REPLACE, builder.build()).enqueue() + enqueue(workManager.beginUniqueWork(key, ExistingWorkPolicy.REPLACE, builder.build())) } override fun cancelNotifications() { diff --git a/app/src/main/java/org/tasks/preferences/fragments/Synchronization.kt b/app/src/main/java/org/tasks/preferences/fragments/Synchronization.kt index 8ca6fe597..6819b4b81 100644 --- a/app/src/main/java/org/tasks/preferences/fragments/Synchronization.kt +++ b/app/src/main/java/org/tasks/preferences/fragments/Synchronization.kt @@ -94,13 +94,17 @@ class Synchronization : InjectingPreferenceFragment() { override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) { if (requestCode == REQUEST_CALDAV_SETTINGS) { if (resultCode == Activity.RESULT_OK) { - workManager.sync(true) - workManager.updateBackgroundSync() + lifecycleScope.launch(NonCancellable) { + workManager.sync(true) + workManager.updateBackgroundSync() + } } } else if (requestCode == REQUEST_GOOGLE_TASKS) { if (resultCode == Activity.RESULT_OK) { - workManager.sync(true) - workManager.updateBackgroundSync() + lifecycleScope.launch(NonCancellable) { + workManager.sync(true) + workManager.updateBackgroundSync() + } } else if (data != null) { toaster.longToast(data.getStringExtra(GtasksLoginActivity.EXTRA_ERROR)) } diff --git a/app/src/main/java/org/tasks/scheduling/RefreshScheduler.kt b/app/src/main/java/org/tasks/scheduling/RefreshScheduler.kt index 062875cd5..ace9e3818 100644 --- a/app/src/main/java/org/tasks/scheduling/RefreshScheduler.kt +++ b/app/src/main/java/org/tasks/scheduling/RefreshScheduler.kt @@ -28,7 +28,7 @@ class RefreshScheduler @Inject internal constructor( } @Synchronized - fun scheduleRefresh(task: Task) { + suspend fun scheduleRefresh(task: Task) { if (task.isCompleted && preferences.getBoolean(R.string.p_temporarily_show_completed_tasks, false)) { scheduleRefresh(task.completionDate + DateUtilities.ONE_MINUTE) @@ -41,7 +41,7 @@ class RefreshScheduler @Inject internal constructor( } @Synchronized - fun scheduleNext() { + suspend fun scheduleNext() { val lapsed = jobs.headSet(DateTimeUtils.currentTimeMillis() + 1).toImmutableList() jobs.removeAll(lapsed) if (!jobs.isEmpty()) { @@ -49,7 +49,7 @@ class RefreshScheduler @Inject internal constructor( } } - private fun scheduleRefresh(timestamp: Long) { + private suspend fun scheduleRefresh(timestamp: Long) { val now = DateTimeUtils.currentTimeMillis() if (now < timestamp) { val upcoming = jobs.tailSet(now)