diff --git a/app/src/main/java/com/bitchat/android/service/PeerAvailabilityNotifier.kt b/app/src/main/java/com/bitchat/android/service/PeerAvailabilityNotifier.kt index de93bda8..0a610c46 100644 --- a/app/src/main/java/com/bitchat/android/service/PeerAvailabilityNotifier.kt +++ b/app/src/main/java/com/bitchat/android/service/PeerAvailabilityNotifier.kt @@ -13,6 +13,7 @@ import android.util.Log import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat import androidx.core.content.ContextCompat +import androidx.core.content.edit import com.bitchat.android.MainActivity import com.bitchat.android.R @@ -22,24 +23,108 @@ internal enum class PeerAvailabilityAction { CLEAR } +internal interface PeerAvailabilityAlertHistory { + var lastAlertAtMillis: Long? +} + +internal class SharedPreferencesPeerAvailabilityAlertHistory( + context: Context +) : PeerAvailabilityAlertHistory { + companion object { + internal const val PREFERENCES_NAME = "peer_availability_notifications" + private const val KEY_LAST_ALERT_AT_MILLIS = "last_alert_at_millis" + } + + private val preferences = + context.getSharedPreferences(PREFERENCES_NAME, Context.MODE_PRIVATE) + + override var lastAlertAtMillis: Long? + get() = if (preferences.contains(KEY_LAST_ALERT_AT_MILLIS)) { + preferences.getLong(KEY_LAST_ALERT_AT_MILLIS, 0L) + } else { + null + } + set(value) { + preferences.edit { + if (value == null) { + remove(KEY_LAST_ALERT_AT_MILLIS) + } else { + putLong(KEY_LAST_ALERT_AT_MILLIS, value) + } + } + } +} + /** - * Tracks mesh availability epochs. A notification is eligible only for a background - * transition from no peers to at least one peer. Returning to zero starts a new epoch. + * Tracks mesh availability epochs with two anti-flapping gates: + * - no more than one alert per persisted cooldown window; + * - after an alert, the mesh must remain empty before another epoch can re-arm. */ -internal class PeerAvailabilityTracker { +internal class PeerAvailabilityTracker( + private val alertHistory: PeerAvailabilityAlertHistory, + private val nowMillis: () -> Long = System::currentTimeMillis, + private val alertCooldownMs: Long = ALERT_COOLDOWN_MS, + private val emptyRearmDelayMs: Long = EMPTY_REARM_DELAY_MS +) { + companion object { + internal const val ALERT_COOLDOWN_MS = 5 * 60_000L + internal const val EMPTY_REARM_DELAY_MS = 30_000L + } + private var previousPeerCount = 0 + private var isArmed = true + private var emptySinceMillis: Long? = null + + init { + require(alertCooldownMs >= 0) { "alertCooldownMs must not be negative" } + require(emptyRearmDelayMs >= 0) { "emptyRearmDelayMs must not be negative" } + } fun update(peerCount: Int, isAppInBackground: Boolean): PeerAvailabilityAction { require(peerCount >= 0) { "peerCount must not be negative" } - val action = when { - peerCount == 0 -> PeerAvailabilityAction.CLEAR - previousPeerCount == 0 && isAppInBackground -> PeerAvailabilityAction.SHOW - else -> PeerAvailabilityAction.NONE + val now = nowMillis() + if (peerCount == 0) { + if (previousPeerCount > 0 || emptySinceMillis == null) { + emptySinceMillis = now + } + previousPeerCount = 0 + return PeerAvailabilityAction.CLEAR } + val transitionedFromEmpty = previousPeerCount == 0 previousPeerCount = peerCount - return action + if (!transitionedFromEmpty) return PeerAvailabilityAction.NONE + + if (!isArmed) { + val emptySince = emptySinceMillis + val remainedEmptyLongEnough = + emptySince != null && now - emptySince >= emptyRearmDelayMs + if (!remainedEmptyLongEnough) { + emptySinceMillis = null + return PeerAvailabilityAction.NONE + } + isArmed = true + } + emptySinceMillis = null + + val lastAlertAt = alertHistory.lastAlertAtMillis + val cooldownElapsed = + lastAlertAt == null || now - lastAlertAt >= alertCooldownMs + return if (isAppInBackground && cooldownElapsed) { + PeerAvailabilityAction.SHOW + } else { + PeerAvailabilityAction.NONE + } + } + + /** + * Called only after NotificationManager accepts the post. Failed or disabled posts + * do not consume the cooldown or require the mesh to re-arm. + */ + fun markAlertShown() { + alertHistory.lastAlertAtMillis = nowMillis() + isArmed = false } } @@ -71,7 +156,9 @@ internal class PeerAvailabilityNotifier( private val context: Context, private val notificationManager: NotificationManagerCompat = NotificationManagerCompat.from(context), - private val tracker: PeerAvailabilityTracker = PeerAvailabilityTracker(), + private val tracker: PeerAvailabilityTracker = PeerAvailabilityTracker( + SharedPreferencesPeerAvailabilityAlertHistory(context) + ), private val textProvider: PeerAvailabilityTextProvider = AndroidPeerAvailabilityTextProvider(context), private val canPostNotifications: () -> Boolean = { @@ -140,6 +227,7 @@ internal class PeerAvailabilityNotifier( .setContentText(textProvider.body(peerCount)) .setContentIntent(pendingIntent) .setAutoCancel(true) + .setOnlyAlertOnce(true) .setPriority(NotificationCompat.PRIORITY_HIGH) .setCategory(NotificationCompat.CATEGORY_SOCIAL) .setShowWhen(true) @@ -148,6 +236,7 @@ internal class PeerAvailabilityNotifier( try { notificationManager.notify(NOTIFICATION_ID, notification) + tracker.markAlertShown() Log.i(TAG, "Posted peer availability notification for $peerCount peer(s)") } catch (error: SecurityException) { Log.w(TAG, "Notification permission changed before peer alert was posted", error) diff --git a/app/src/test/kotlin/com/bitchat/android/service/PeerAvailabilityNotifierTest.kt b/app/src/test/kotlin/com/bitchat/android/service/PeerAvailabilityNotifierTest.kt index 68f4aa25..8e2a0937 100644 --- a/app/src/test/kotlin/com/bitchat/android/service/PeerAvailabilityNotifierTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/service/PeerAvailabilityNotifierTest.kt @@ -1,11 +1,13 @@ package com.bitchat.android.service +import android.app.Notification import android.app.NotificationManager import android.content.Context import androidx.test.core.app.ApplicationProvider import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -16,19 +18,82 @@ import org.robolectric.annotation.Config class PeerAvailabilityTrackerTest { @Test - fun `background zero to nonzero transition shows once per availability epoch`() { - val tracker = PeerAvailabilityTracker() + fun `repeated flapping produces only one alert during cooldown`() { + val clock = MutableClock() + val tracker = tracker(clock = clock) + var showCount = 0 + + if (tracker.update(1, isAppInBackground = true) == PeerAvailabilityAction.SHOW) { + showCount++ + tracker.markAlertShown() + } + + repeat(5) { + clock.advance(1_000L) + assertEquals( + PeerAvailabilityAction.CLEAR, + tracker.update(0, isAppInBackground = true) + ) + clock.advance(PeerAvailabilityTracker.EMPTY_REARM_DELAY_MS) + if (tracker.update(1, isAppInBackground = true) == PeerAvailabilityAction.SHOW) { + showCount++ + tracker.markAlertShown() + } + } + + assertEquals(1, showCount) + } + + @Test + fun `mesh must remain empty before notification re-arms`() { + val clock = MutableClock() + val tracker = tracker( + clock = clock, + alertCooldownMs = 0L + ) + + assertEquals(PeerAvailabilityAction.SHOW, tracker.update(1, isAppInBackground = true)) + tracker.markAlertShown() assertEquals(PeerAvailabilityAction.CLEAR, tracker.update(0, isAppInBackground = true)) - assertEquals(PeerAvailabilityAction.SHOW, tracker.update(1, isAppInBackground = true)) - assertEquals(PeerAvailabilityAction.NONE, tracker.update(2, isAppInBackground = true)) + clock.advance(PeerAvailabilityTracker.EMPTY_REARM_DELAY_MS - 1) + assertEquals(PeerAvailabilityAction.NONE, tracker.update(1, isAppInBackground = true)) + assertEquals(PeerAvailabilityAction.CLEAR, tracker.update(0, isAppInBackground = true)) + clock.advance(PeerAvailabilityTracker.EMPTY_REARM_DELAY_MS) assertEquals(PeerAvailabilityAction.SHOW, tracker.update(1, isAppInBackground = true)) } + @Test + fun `cooldown survives tracker recreation`() { + val clock = MutableClock() + val history = InMemoryAlertHistory() + val originalTracker = tracker(clock = clock, history = history) + + assertEquals( + PeerAvailabilityAction.SHOW, + originalTracker.update(1, isAppInBackground = true) + ) + originalTracker.markAlertShown() + + clock.advance(PeerAvailabilityTracker.ALERT_COOLDOWN_MS - 1) + val restartedDuringCooldown = tracker(clock = clock, history = history) + assertEquals( + PeerAvailabilityAction.NONE, + restartedDuringCooldown.update(1, isAppInBackground = true) + ) + + clock.advance(1) + val restartedAfterCooldown = tracker(clock = clock, history = history) + assertEquals( + PeerAvailabilityAction.SHOW, + restartedAfterCooldown.update(1, isAppInBackground = true) + ) + } + @Test fun `foreground discovery is not replayed after app enters background`() { - val tracker = PeerAvailabilityTracker() + val tracker = tracker() assertEquals(PeerAvailabilityAction.NONE, tracker.update(1, isAppInBackground = false)) assertEquals(PeerAvailabilityAction.NONE, tracker.update(1, isAppInBackground = true)) @@ -38,7 +103,19 @@ class PeerAvailabilityTrackerTest { @Test(expected = IllegalArgumentException::class) fun `negative peer count is rejected`() { - PeerAvailabilityTracker().update(-1, isAppInBackground = true) + tracker().update(-1, isAppInBackground = true) + } + + private fun tracker( + clock: MutableClock = MutableClock(), + history: InMemoryAlertHistory = InMemoryAlertHistory(), + alertCooldownMs: Long = PeerAvailabilityTracker.ALERT_COOLDOWN_MS + ): PeerAvailabilityTracker { + return PeerAvailabilityTracker( + alertHistory = history, + nowMillis = clock::now, + alertCooldownMs = alertCooldownMs + ) } } @@ -54,6 +131,10 @@ class PeerAvailabilityNotifierTest { systemNotificationManager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager systemNotificationManager.cancelAll() + context.getSharedPreferences( + SharedPreferencesPeerAvailabilityAlertHistory.PREFERENCES_NAME, + Context.MODE_PRIVATE + ).edit().clear().commit() } @Test @@ -61,6 +142,7 @@ class PeerAvailabilityNotifierTest { val textProvider = testTextProvider() val notifier = PeerAvailabilityNotifier( context = context, + tracker = tracker(), textProvider = textProvider, canPostNotifications = { true } ) @@ -69,7 +151,9 @@ class PeerAvailabilityNotifierTest { notifier.onPeerCountChanged(2, isAppInBackground = true) val notification = - shadowOf(systemNotificationManager).getNotification(PeerAvailabilityNotifier.NOTIFICATION_ID) + shadowOf(systemNotificationManager).getNotification( + PeerAvailabilityNotifier.NOTIFICATION_ID + ) assertNotNull(notification) assertEquals(PeerAvailabilityNotifier.CHANNEL_ID, notification.channelId) assertEquals( @@ -80,6 +164,7 @@ class PeerAvailabilityNotifierTest { textProvider.body(2), notification.extras.getString("android.text") ) + assertTrue(notification.flags and Notification.FLAG_ONLY_ALERT_ONCE != 0) assertNotNull( systemNotificationManager.getNotificationChannel(PeerAvailabilityNotifier.CHANNEL_ID) ) @@ -89,6 +174,7 @@ class PeerAvailabilityNotifierTest { fun `returning to zero cancels availability notification`() { val notifier = PeerAvailabilityNotifier( context = context, + tracker = tracker(), textProvider = testTextProvider(), canPostNotifications = { true } ) @@ -97,14 +183,18 @@ class PeerAvailabilityNotifierTest { notifier.onPeerCountChanged(0, isAppInBackground = true) assertNull( - shadowOf(systemNotificationManager).getNotification(PeerAvailabilityNotifier.NOTIFICATION_ID) + shadowOf(systemNotificationManager).getNotification( + PeerAvailabilityNotifier.NOTIFICATION_ID + ) ) } @Test fun `disabled notifications do not post`() { + val history = InMemoryAlertHistory() val notifier = PeerAvailabilityNotifier( context = context, + tracker = PeerAvailabilityTracker(history), textProvider = testTextProvider(), canPostNotifications = { false } ) @@ -112,8 +202,26 @@ class PeerAvailabilityNotifierTest { notifier.onPeerCountChanged(1, isAppInBackground = true) assertNull( - shadowOf(systemNotificationManager).getNotification(PeerAvailabilityNotifier.NOTIFICATION_ID) + shadowOf(systemNotificationManager).getNotification( + PeerAvailabilityNotifier.NOTIFICATION_ID + ) ) + assertNull(history.lastAlertAtMillis) + } + + @Test + fun `shared preferences history persists last alert time`() { + val history = SharedPreferencesPeerAvailabilityAlertHistory(context) + history.lastAlertAtMillis = 42L + + assertEquals( + 42L, + SharedPreferencesPeerAvailabilityAlertHistory(context).lastAlertAtMillis ?: -1L + ) + } + + private fun tracker(): PeerAvailabilityTracker { + return PeerAvailabilityTracker(InMemoryAlertHistory()) } private fun testTextProvider(): PeerAvailabilityTextProvider { @@ -124,3 +232,17 @@ class PeerAvailabilityNotifierTest { } } } + +private class InMemoryAlertHistory( + override var lastAlertAtMillis: Long? = null +) : PeerAvailabilityAlertHistory + +private class MutableClock( + private var currentMillis: Long = 1_000L +) { + fun now(): Long = currentMillis + + fun advance(durationMillis: Long) { + currentMillis += durationMillis + } +}