diff --git a/app/src/main/java/org/tasks/notifications/NotificationLimiter.java b/app/src/main/java/org/tasks/notifications/NotificationLimiter.java index 314e7ca54..d281c9dc9 100644 --- a/app/src/main/java/org/tasks/notifications/NotificationLimiter.java +++ b/app/src/main/java/org/tasks/notifications/NotificationLimiter.java @@ -29,6 +29,12 @@ class NotificationLimiter { return evicted; } + synchronized void remove(Iterable ids) { + for (Long id : ids) { + remove(id); + } + } + synchronized void remove(long id) { if (id == NotificationManager.SUMMARY_NOTIFICATION_ID) { summary = false; diff --git a/app/src/main/java/org/tasks/notifications/NotificationManager.kt b/app/src/main/java/org/tasks/notifications/NotificationManager.kt index 1d604cff5..077791c59 100644 --- a/app/src/main/java/org/tasks/notifications/NotificationManager.kt +++ b/app/src/main/java/org/tasks/notifications/NotificationManager.kt @@ -5,7 +5,6 @@ import android.app.PendingIntent import android.content.Context import android.content.Intent import androidx.core.app.NotificationCompat -import androidx.core.app.NotificationManagerCompat import com.todoroo.andlib.utility.AndroidUtilities import com.todoroo.andlib.utility.DateUtilities import com.todoroo.astrid.reminders.ReminderService @@ -38,10 +37,9 @@ class NotificationManager @Inject constructor( private val taskDao: TaskDao, private val locationDao: LocationDao, private val localBroadcastManager: LocalBroadcastManager, - private val reminderService: ReminderService) { - private val notificationManagerCompat = NotificationManagerCompat.from(context) + private val reminderService: ReminderService, + private val notificationManager: ThrottledNotificationManager) { private val colorProvider = ColorProvider(context, preferences) - private val throttle = Throttle(NOTIFICATIONS_PER_SECOND, tag = "NOTIFY") private val queue = NotificationLimiter(MAX_NOTIFICATIONS) @SuppressLint("CheckResult") @@ -56,11 +54,9 @@ class NotificationManager @Inject constructor( @SuppressLint("CheckResult") suspend fun cancel(ids: Iterable) { for (id in ids) { - throttle.run { - notificationManagerCompat.cancel(id.toInt()) - queue.remove(id) - } + notificationManager.cancel(id.toInt()) } + queue.remove(ids) notificationDao.deleteAll(ids.toList()) notifyTasks(emptyList(), alert = false, nonstop = false, fiveTimes = false) } @@ -69,7 +65,7 @@ class NotificationManager @Inject constructor( val notifications = notificationDao.getAllOrdered() if (cancelExisting) { for (notification in notifications) { - notificationManagerCompat.cancel(notification.taskId.toInt()) + notificationManager.cancel(notification.taskId.toInt()) } } if (preferences.bundleNotifications() && notifications.size > 1) { @@ -150,7 +146,7 @@ class NotificationManager @Inject constructor( for (notification in notifications) { val builder = getTaskNotification(notification) if (builder == null) { - notificationManagerCompat.cancel(notification.taskId.toInt()) + notificationManager.cancel(notification.taskId.toInt()) notificationDao.delete(notification.taskId) } else { builder @@ -206,7 +202,7 @@ class NotificationManager @Inject constructor( cancel(evicted) } for (i in 0 until ringTimes) { - throttle.run { notificationManagerCompat.notify(notificationId.toInt(), notification) } + notificationManager.notify(notificationId.toInt(), notification) } } @@ -365,7 +361,7 @@ class NotificationManager @Inject constructor( } private fun cancelSummaryNotification() { - notificationManagerCompat.cancel(SUMMARY_NOTIFICATION_ID) + notificationManager.cancel(SUMMARY_NOTIFICATION_ID) } companion object { @@ -377,7 +373,5 @@ class NotificationManager @Inject constructor( const val EXTRA_NOTIFICATION_ID = "extra_notification_id" const val SUMMARY_NOTIFICATION_ID = 0 private const val GROUP_KEY = "tasks" - private const val NOTIFICATIONS_PER_SECOND = 4 } - } \ No newline at end of file diff --git a/app/src/main/java/org/tasks/notifications/Throttle.kt b/app/src/main/java/org/tasks/notifications/Throttle.kt index 8ae75c74a..0e472cfd7 100644 --- a/app/src/main/java/org/tasks/notifications/Throttle.kt +++ b/app/src/main/java/org/tasks/notifications/Throttle.kt @@ -1,33 +1,37 @@ package org.tasks.notifications -import kotlinx.coroutines.* +import kotlinx.coroutines.runBlocking import org.tasks.time.DateTimeUtils.currentTimeMillis import timber.log.Timber +import java.util.concurrent.Executor import java.util.concurrent.Executors.newSingleThreadExecutor internal class Throttle constructor( ratePerPeriod: Int, private val periodMillis: Long = 1000, private val tag: String = "", - private val scope: CoroutineScope = - CoroutineScope(newSingleThreadExecutor().asCoroutineDispatcher() + SupervisorJob()), - private val sleeper: suspend (Long) -> Unit = { delay(it) }) { + private val executor: Executor = newSingleThreadExecutor(), + private val sleeper: (Long) -> Unit = { Thread.sleep(it) }) { private val throttle: LongArray = LongArray(ratePerPeriod) private var oldest = 0 @Synchronized - fun run(runnable: suspend () -> Unit): Job = scope.launch { - val sleep = throttle[oldest] - (currentTimeMillis() - periodMillis) - if (sleep > 0) { - Timber.v("$tag: Throttled for ${sleep}ms") - sleeper.invoke(sleep) + fun run(runnable: suspend () -> Unit) { + executor.execute { + val sleep = throttle[oldest] - (currentTimeMillis() - periodMillis) + if (sleep > 0) { + Timber.v("$tag: Throttled for ${sleep}ms") + sleeper.invoke(sleep) + } + try { + runBlocking { + runnable.invoke() + } + } catch (e: Exception) { + Timber.e(e) + } + throttle[oldest] = currentTimeMillis() + oldest = (oldest + 1) % throttle.size } - try { - runnable.invoke() - } catch (e: Exception) { - Timber.e(e) - } - throttle[oldest] = currentTimeMillis() - oldest = (oldest + 1) % throttle.size } } \ No newline at end of file diff --git a/app/src/main/java/org/tasks/notifications/ThrottledNotificationManager.kt b/app/src/main/java/org/tasks/notifications/ThrottledNotificationManager.kt new file mode 100644 index 000000000..4ec4c730c --- /dev/null +++ b/app/src/main/java/org/tasks/notifications/ThrottledNotificationManager.kt @@ -0,0 +1,32 @@ +package org.tasks.notifications + +import android.app.Notification +import android.content.Context +import androidx.core.app.NotificationManagerCompat +import dagger.hilt.android.qualifiers.ApplicationContext +import java.util.concurrent.Executors.newSingleThreadExecutor +import javax.inject.Inject + +class ThrottledNotificationManager @Inject constructor( + @ApplicationContext val context: Context +) { + private val notificationManagerCompat = NotificationManagerCompat.from(context) + private val executor = newSingleThreadExecutor() + private val throttle = Throttle(NOTIFICATIONS_PER_SECOND, executor = executor, tag = "NOTIFY") + + fun cancel(id: Int) { + executor.execute { + notificationManagerCompat.cancel(id) + } + } + + fun notify(id: Int, notification: Notification) { + throttle.run { + notificationManagerCompat.notify(id, notification) + } + } + + companion object { + private const val NOTIFICATIONS_PER_SECOND = 4 + } +} \ No newline at end of file diff --git a/app/src/test/java/org/tasks/notifications/ThrottleTest.kt b/app/src/test/java/org/tasks/notifications/ThrottleTest.kt index bfea969e5..9d1e9dfa8 100644 --- a/app/src/test/java/org/tasks/notifications/ThrottleTest.kt +++ b/app/src/test/java/org/tasks/notifications/ThrottleTest.kt @@ -1,12 +1,12 @@ package org.tasks.notifications +import androidx.work.impl.utils.SynchronousExecutor import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.test.runBlockingTest import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test -import org.tasks.SuspendFreeze +import org.tasks.Freeze import org.tasks.time.DateTimeUtils @ExperimentalCoroutinesApi @@ -20,8 +20,8 @@ class ThrottleTest { } @Test - fun dontThrottle() = runBlockingTest { - throttle = Throttle(3, scope = this) { sleep.add(it) } + fun dontThrottle() { + throttle = Throttle(3, executor = SynchronousExecutor()) { sleep.add(it) } val now = DateTimeUtils.currentTimeMillis() runAt(now) runAt(now) @@ -31,8 +31,8 @@ class ThrottleTest { } @Test - fun throttleForOneMillisecond() = runBlockingTest { - throttle = Throttle(3, scope = this) { sleep.add(it) } + fun throttleForOneMillisecond() { + throttle = Throttle(3, executor = SynchronousExecutor()) { sleep.add(it) } val now = DateTimeUtils.currentTimeMillis() runAt(now) runAt(now) @@ -42,8 +42,8 @@ class ThrottleTest { } @Test - fun throttleForOneSecond() = runBlockingTest { - throttle = Throttle(3, scope = this) { sleep.add(it) } + fun throttleForOneSecond() { + throttle = Throttle(3, executor = SynchronousExecutor()) { sleep.add(it) } val now = DateTimeUtils.currentTimeMillis() runAt(now) runAt(now) @@ -53,8 +53,8 @@ class ThrottleTest { } @Test - fun throttleMultiple() = runBlockingTest { - throttle = Throttle(3, scope = this) { sleep.add(it) } + fun throttleMultiple() { + throttle = Throttle(3, executor = SynchronousExecutor()) { sleep.add(it) } val now = DateTimeUtils.currentTimeMillis() runAt(now) runAt(now + 200) @@ -65,7 +65,7 @@ class ThrottleTest { assertEquals(arrayListOf(300L, 450L), sleep) } - private suspend fun runAt(millis: Long) { - SuspendFreeze.freezeAt(millis) { throttle.run {} } + private fun runAt(millis: Long) { + Freeze.freezeAt(millis) { throttle.run {} } } } \ No newline at end of file