From 2179fe748ad4e95f32492aa1fc98a7d0219ff34c Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Mon, 14 Jan 2019 16:31:25 -0600 Subject: [PATCH] Limit notifications to five per second --- app/build.gradle | 1 + .../org/tasks/notifications/ThrottleTest.java | 86 +++++++++++++++++++ .../andlib/utility/AndroidUtilities.java | 14 +++ .../notifications/NotificationManager.java | 4 +- .../org/tasks/notifications/Throttle.java | 41 +++++++++ 5 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 app/src/androidTest/java/org/tasks/notifications/ThrottleTest.java create mode 100644 app/src/main/java/org/tasks/notifications/Throttle.java diff --git a/app/build.gradle b/app/build.gradle index 3b4d6f059..6f0365c17 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -190,6 +190,7 @@ dependencies { androidTestImplementation 'com.natpryce:make-it-easy:4.0.1' androidTestImplementation 'androidx.test:runner:1.1.1' androidTestImplementation 'androidx.test:rules:1.1.1' + androidTestImplementation 'androidx.test.ext:junit:1.1.0' androidTestImplementation 'androidx.annotation:annotation:1.0.1' } diff --git a/app/src/androidTest/java/org/tasks/notifications/ThrottleTest.java b/app/src/androidTest/java/org/tasks/notifications/ThrottleTest.java new file mode 100644 index 000000000..deda67ebd --- /dev/null +++ b/app/src/androidTest/java/org/tasks/notifications/ThrottleTest.java @@ -0,0 +1,86 @@ +package org.tasks.notifications; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.tasks.Freeze.freezeAt; +import static org.tasks.time.DateTimeUtils.currentTimeMillis; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.tasks.notifications.Throttle.Sleeper; + +@RunWith(AndroidJUnit4.class) +public class ThrottleTest { + + private Sleeper sleeper; + private Throttle throttle; + + @Before + public void setUp() { + sleeper = mock(Sleeper.class); + throttle = new Throttle(3, sleeper); + } + + @After + public void tearDown() { + verifyNoMoreInteractions(sleeper); + } + + @Test + public void dontThrottle() { + long now = currentTimeMillis(); + + runAt(now); + runAt(now); + runAt(now); + runAt(now + 1000); + } + + @Test + public void throttleForOneMillisecond() { + long now = currentTimeMillis(); + + runAt(now); + runAt(now); + runAt(now); + runAt(now + 999); + + verify(sleeper).sleep(1); + } + + @Test + public void throttleForOneSecond() { + long now = currentTimeMillis(); + + runAt(now); + runAt(now); + runAt(now); + runAt(now); + + verify(sleeper).sleep(1000); + } + + @Test + public void throttleMultiple() { + long now = currentTimeMillis(); + + runAt(now); + runAt(now + 200); + runAt(now + 600); + runAt(now + 700); + + verify(sleeper).sleep(300); + + runAt(now + 750); + + verify(sleeper).sleep(450); + } + + private void runAt(long millis) { + freezeAt(millis).thawAfter(() -> throttle.run(() -> {})); + } +} diff --git a/app/src/main/java/com/todoroo/andlib/utility/AndroidUtilities.java b/app/src/main/java/com/todoroo/andlib/utility/AndroidUtilities.java index 2346bc08b..48614bc87 100644 --- a/app/src/main/java/com/todoroo/andlib/utility/AndroidUtilities.java +++ b/app/src/main/java/com/todoroo/andlib/utility/AndroidUtilities.java @@ -11,6 +11,7 @@ import android.content.Context; import android.os.Build; import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; +import android.os.Looper; import android.text.InputType; import android.util.DisplayMetrics; import android.view.View; @@ -19,6 +20,7 @@ import android.widget.TextView; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; +import org.tasks.BuildConfig; import timber.log.Timber; /** @@ -170,6 +172,18 @@ public class AndroidUtilities { return Build.VERSION.SDK_INT >= Build.VERSION_CODES.O; } + public static void assertNotMainThread() { + if (BuildConfig.DEBUG && isMainThread()) { + throw new IllegalStateException(); + } + } + + private static boolean isMainThread() { + return atLeastMarshmallow() + ? Looper.getMainLooper().isCurrentThread() + : Thread.currentThread() == Looper.getMainLooper().getThread(); + } + /** * Sleep, ignoring interruption. Before using this method, think carefully about why you are * ignoring interruptions. diff --git a/app/src/main/java/org/tasks/notifications/NotificationManager.java b/app/src/main/java/org/tasks/notifications/NotificationManager.java index 0e3d44186..e5dff66de 100644 --- a/app/src/main/java/org/tasks/notifications/NotificationManager.java +++ b/app/src/main/java/org/tasks/notifications/NotificationManager.java @@ -61,6 +61,7 @@ public class NotificationManager { static final String EXTRA_NOTIFICATION_ID = "extra_notification_id"; private static final String GROUP_KEY = "tasks"; private static final int SUMMARY_NOTIFICATION_ID = 0; + private static final int NOTIFICATIONS_PER_SECOND = 5; private final NotificationManagerCompat notificationManagerCompat; private final LocationDao locationDao; private final NotificationDao notificationDao; @@ -68,6 +69,7 @@ public class NotificationManager { private final Context context; private final Preferences preferences; private final CheckBoxes checkBoxes; + private final Throttle throttle = new Throttle(NOTIFICATIONS_PER_SECOND); @Inject public NotificationManager( @@ -254,7 +256,7 @@ public class NotificationManager { PendingIntent.getBroadcast( context, (int) notificationId, deleteIntent, PendingIntent.FLAG_UPDATE_CURRENT); for (int i = 0; i < ringTimes; i++) { - notificationManagerCompat.notify((int) notificationId, notification); + throttle.run(() -> notificationManagerCompat.notify((int) notificationId, notification)); } } diff --git a/app/src/main/java/org/tasks/notifications/Throttle.java b/app/src/main/java/org/tasks/notifications/Throttle.java new file mode 100644 index 000000000..29acd0f27 --- /dev/null +++ b/app/src/main/java/org/tasks/notifications/Throttle.java @@ -0,0 +1,41 @@ +package org.tasks.notifications; + +import static com.todoroo.andlib.utility.AndroidUtilities.assertNotMainThread; +import static org.tasks.time.DateTimeUtils.currentTimeMillis; + +class Throttle { + private final long[] throttle; + private final Sleeper sleeper; + private int oldest = 0; + + Throttle(int ratePerSecond) { + this(ratePerSecond, Throttle::sleep); + } + + Throttle(int ratePerSecond, Sleeper sleeper) { + this.sleeper = sleeper; + throttle = new long[ratePerSecond]; + } + + private static void sleep(long millis) { + try { + Thread.sleep(millis); + } catch (InterruptedException ignored) { + } + } + + synchronized void run(Runnable runnable) { + assertNotMainThread(); + long sleep = throttle[oldest] - (currentTimeMillis() - 1000); + if (sleep > 0) { + sleeper.sleep(sleep); + } + runnable.run(); + throttle[oldest] = currentTimeMillis(); + oldest = (oldest + 1) % throttle.length; + } + + public interface Sleeper { + void sleep(long millis); + } +}