diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRelayService.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRelayService.kt index 3411379e4c..d90c812658 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRelayService.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationRelayService.kt @@ -373,6 +373,16 @@ class NotificationRelayService : Service() { if (fresh.isNotEmpty()) lastBreakdown = fresh val breakdown = fresh.ifEmpty { lastBreakdown }.takeIf { it.isNotEmpty() } + // Deliberately left ungrouped. This notification is ongoing and IMPORTANCE_LOW, so it + // sits in the shade's Silent section next to the low-importance content kinds + // (reactions, reposts) — and Android 16 sweeps everything ungrouped in a section into + // one aggregate bundle whose summary inherits FLAG_ONGOING_EVENT from any child that + // has it, making the whole bundle un-swipeable. Giving this one a group of its own + // would not help: a group with a summary but no children, or a child with no summary, + // is force-grouped just the same. What keeps content notifications out of that bundle + // is that they always post their own group summary (see NotificationUtils), which + // leaves this the only ungrouped silent notification we post — one is below the + // threshold, so no bundle is formed and nothing gets stapled to it. return NotificationCompat .Builder(this, CHANNEL_ID) .setContentTitle(getString(R.string.always_on_notif_title)) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt index ddb16c72cb..26b2930f02 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationReplyReceiver.kt @@ -30,6 +30,8 @@ import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.LocalPreferences import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.accountsCache.AccountCacheState +import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.cancelAndPrune +import com.vitorpamplona.amethyst.service.notifications.NotificationUtils.cancelChildlessGroupSummaries import com.vitorpamplona.amethyst.ui.actions.NewMessageTagger import com.vitorpamplona.quartz.nip01Core.hints.EventHintBundle import com.vitorpamplona.quartz.nip01Core.tags.people.PTag @@ -54,13 +56,35 @@ class NotificationReplyReceiver : BroadcastReceiver() { intent: Intent, ) { val notificationId = intent.getIntExtra(NotificationUtils.KEY_NOTIFICATION_ID, 0) + + // Whatever the action, the user is done with this notification, so record it before + // doing anything else. An enrichment window may still be open on the event (up to + // 25s from the first post), and it re-posts the notification every time metadata + // lands — without this the notification the user just dealt with comes back, and the + // enricher keeps a relay subscription and a wakelock alive for it until the window + // elapses. Replies mark it after the send succeeds instead, so a failure leaves the + // notification to enrich and retry. + val eventId = intent.getStringExtra(NotificationUtils.KEY_EVENT_ID) + if (intent.action != NotificationUtils.REPLY_ACTION && + intent.action != NotificationUtils.PUBLIC_REPLY_ACTION && + intent.action != NotificationUtils.MARMOT_REPLY_ACTION + ) { + eventId?.let { NotificationUtils.markDismissed(it) } + } + val notificationManager = ContextCompat.getSystemService(context, NotificationManager::class.java) as NotificationManager when (intent.action) { NotificationUtils.MARK_READ_ACTION -> { - notificationManager.cancel(notificationId) + notificationManager.cancelAndPrune(notificationId) + } + + // The user swiped the notification away. It is already gone; all that is left + // is to take its group summary with it when it was the last child. + NotificationUtils.DISMISS_ACTION -> { + notificationManager.cancelChildlessGroupSummaries(alreadyGone = notificationId) } NotificationUtils.REPLY_ACTION -> { @@ -78,7 +102,7 @@ class NotificationReplyReceiver : BroadcastReceiver() { if (members.isEmpty()) return - runOnRelay(notificationManager, notificationId) { + runOnRelay(notificationManager, notificationId, eventId) { sendReply(accountNpub, members, replyText) } } @@ -95,7 +119,7 @@ class NotificationReplyReceiver : BroadcastReceiver() { val accountNpub = intent.getStringExtra(NotificationUtils.KEY_ACCOUNT_NPUB) ?: return val targetEventId = intent.getStringExtra(NotificationUtils.KEY_TARGET_EVENT_ID) ?: return - runOnRelay(notificationManager, notificationId) { + runOnRelay(notificationManager, notificationId, eventId) { sendPublicReply(accountNpub, targetEventId, replyText) } } @@ -114,7 +138,7 @@ class NotificationReplyReceiver : BroadcastReceiver() { val replyToInnerId = intent.getStringExtra(NotificationUtils.KEY_MARMOT_REPLY_TO_INNER_ID) val replyToInnerAuthor = intent.getStringExtra(NotificationUtils.KEY_MARMOT_REPLY_TO_INNER_AUTHOR) - runOnRelay(notificationManager, notificationId) { + runOnRelay(notificationManager, notificationId, eventId) { sendMarmotReply(accountNpub, nostrGroupId, replyToInnerId, replyToInnerAuthor, replyText) } } @@ -124,6 +148,7 @@ class NotificationReplyReceiver : BroadcastReceiver() { private fun runOnRelay( notificationManager: NotificationManager, notificationId: Int, + eventId: String?, block: suspend () -> Unit, ) { val pendingResult = goAsync() @@ -138,7 +163,8 @@ class NotificationReplyReceiver : BroadcastReceiver() { try { block() - notificationManager.cancel(notificationId) + eventId?.let { NotificationUtils.markDismissed(it) } + notificationManager.cancelAndPrune(notificationId) } catch (e: Exception) { if (e is CancellationException) throw e Log.e("NotificationReply") { "Failed to send reply: ${e.message}" } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt index a1879a70f4..6d4791386c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/notifications/NotificationUtils.kt @@ -70,8 +70,16 @@ object NotificationUtils { const val PUBLIC_REPLY_ACTION = "com.vitorpamplona.amethyst.PUBLIC_REPLY_ACTION" const val MARMOT_REPLY_ACTION = "com.vitorpamplona.amethyst.MARMOT_REPLY_ACTION" const val MARK_READ_ACTION = "com.vitorpamplona.amethyst.MARK_READ_ACTION" + const val DISMISS_ACTION = "com.vitorpamplona.amethyst.DISMISS_ACTION" const val KEY_REPLY_TEXT = "key_reply_text" const val KEY_NOTIFICATION_ID = "key_notification_id" + + /** + * Hex id of the event this notification was posted for, carried on every action + * and on the delete intent so the receiver can mark it dismissed. Distinct from + * [KEY_TARGET_EVENT_ID], which is the note an inline reply is addressed to. + */ + const val KEY_EVENT_ID = "key_event_id" const val KEY_ACCOUNT_NPUB = "key_account_npub" const val KEY_CHATROOM_MEMBERS = "key_chatroom_members" const val KEY_TARGET_EVENT_ID = "key_target_event_id" @@ -82,16 +90,27 @@ object NotificationUtils { const val REPLY_GROUP_KEY_PREFIX = "com.vitorpamplona.amethyst.REPLY_NOTIFICATION" private const val REPLY_SUMMARY_ID_BASE = 0x50000 - // Event ids the user has just read/dismissed in-app. The enrichment path - // re-posts a notification as metadata arrives; without this guard a - // notification the user already dismissed would be resurrected seconds later - // when its author's kind:0 lands. Keyed by the event id string (not the - // hashCode) so distinct events can't collide. Entries self-expire after a - // window comfortably longer than the 25s enrichment window. + /** + * Every group key this object posts under starts with this. Used to tell our own + * summaries apart from the ones the system creates when it force-groups us (those + * live under `userId|pkg|g:Aggregate_…`), so the cleanup below never fights the + * platform over a bundle it owns. + */ + private const val OWN_GROUP_PREFIX = "com.vitorpamplona.amethyst." + + // Event ids the user is done with. The enrichment path re-posts a notification + // as metadata arrives; without this guard a notification the user already got + // rid of would be resurrected seconds later when its author's kind:0 lands, and + // the enricher would go on holding a relay window and a wakelock open for it. + // Every way a user can be done with a notification has to record here — reading + // the event in-app, swiping the notification away, "mark as read", and replying + // inline — or that path leaks the resurrection. Keyed by the event id string + // (not the hashCode) so distinct events can't collide. Entries self-expire after + // a window comfortably longer than the 25s enrichment window. private const val DISMISS_GUARD_MS = 90_000L private val recentlyDismissed = ConcurrentHashMap() - private fun markDismissed(eventId: String) { + fun markDismissed(eventId: String) { val now = SystemClock.elapsedRealtime() recentlyDismissed[eventId] = now + DISMISS_GUARD_MS if (recentlyDismissed.size > 256) { @@ -218,6 +237,7 @@ object NotificationUtils { .setPriority(category.priority()) .setCategory(NotificationCompat.CATEGORY_SOCIAL) .setGroup(groupKey) + .setDeleteIntent(dismissIntent(applicationContext, notId, id)) .setAutoCancel(true) .setOnlyAlertOnce(true) .setWhen(time * 1000) @@ -236,11 +256,11 @@ object NotificationUtils { } if (inlineReply != null) { - builder.addAction(publicReplyAction(applicationContext, notId, inlineReply)) + builder.addAction(publicReplyAction(applicationContext, notId, id, inlineReply)) } notify(notId, builder.build()) - sendGroupSummary(category, groupKey, summaryId, applicationContext) + sendGroupSummary(category, groupKey, summaryId, time, applicationContext) } // --------------------------------------------------------------------- @@ -335,20 +355,21 @@ object NotificationUtils { .setPriority(category.priority()) .setCategory(NotificationCompat.CATEGORY_MESSAGE) .setGroup(groupKey) + .setDeleteIntent(dismissIntent(applicationContext, notId, id)) .setAutoCancel(true) .setOnlyAlertOnce(true) .setWhen(time * 1000) when (replyAction) { - is ReplyAction.Dm -> builder.addAction(dmReplyAction(applicationContext, notId, replyAction)) - is ReplyAction.Marmot -> builder.addAction(marmotReplyAction(applicationContext, notId, replyAction)) - null -> publicInlineReply?.let { builder.addAction(publicReplyAction(applicationContext, notId, it)) } + is ReplyAction.Dm -> builder.addAction(dmReplyAction(applicationContext, notId, id, replyAction)) + is ReplyAction.Marmot -> builder.addAction(marmotReplyAction(applicationContext, notId, id, replyAction)) + null -> publicInlineReply?.let { builder.addAction(publicReplyAction(applicationContext, notId, id, it)) } } - if (addMarkRead) builder.addAction(markReadAction(applicationContext, notId)) + if (addMarkRead) builder.addAction(markReadAction(applicationContext, notId, id)) notify(notId, builder.build()) - sendGroupSummary(category, groupKey, summaryId, applicationContext) + sendGroupSummary(category, groupKey, summaryId, time, applicationContext) } // --------------------------------------------------------------------- @@ -370,6 +391,38 @@ object NotificationUtils { ) } + /** + * Fires when the user swipes this notification away (or hits "Clear all"), so the + * group summary can follow its last child out. + * + * We can't rely on the shade to take the summary with it: SystemUI hides a group + * with a single child and renders that child at the top level + * (`ShadeListBuilder.MIN_CHILDREN_FOR_GROUP`), and once promoted the child no + * longer counts as "the only child in its group", so dismissing it leaves our + * summary behind. A childless summary is not harmless — SystemUI promotes it into + * the shade on its own, and the system force-groups it + * (`GroupHelper.isGroupSummaryWithoutChildren`) into the same aggregate bundle we + * post summaries to stay out of. + */ + private fun dismissIntent( + applicationContext: Context, + notId: Int, + eventId: String, + ): PendingIntent { + val intent = + Intent(applicationContext, NotificationReplyReceiver::class.java).apply { + action = DISMISS_ACTION + putExtra(KEY_NOTIFICATION_ID, notId) + putExtra(KEY_EVENT_ID, eventId) + } + return PendingIntent.getBroadcast( + applicationContext, + notId + 2, + intent, + PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT, + ) + } + private fun replyRemoteInput(applicationContext: Context): RemoteInput = RemoteInput .Builder(KEY_REPLY_TEXT) @@ -399,12 +452,14 @@ object NotificationUtils { private fun dmReplyAction( applicationContext: Context, notId: Int, + eventId: String, action: ReplyAction.Dm, ): NotificationCompat.Action { val intent = Intent(applicationContext, NotificationReplyReceiver::class.java).apply { this.action = REPLY_ACTION putExtra(KEY_NOTIFICATION_ID, notId) + putExtra(KEY_EVENT_ID, eventId) putExtra(KEY_ACCOUNT_NPUB, action.accountNpub) putExtra(KEY_CHATROOM_MEMBERS, action.chatroomMembers) } @@ -414,12 +469,14 @@ object NotificationUtils { private fun marmotReplyAction( applicationContext: Context, notId: Int, + eventId: String, action: ReplyAction.Marmot, ): NotificationCompat.Action { val intent = Intent(applicationContext, NotificationReplyReceiver::class.java).apply { this.action = MARMOT_REPLY_ACTION putExtra(KEY_NOTIFICATION_ID, notId) + putExtra(KEY_EVENT_ID, eventId) putExtra(KEY_ACCOUNT_NPUB, action.accountNpub) putExtra(KEY_MARMOT_GROUP_ID, action.nostrGroupId) action.replyToInnerEventId?.let { putExtra(KEY_MARMOT_REPLY_TO_INNER_ID, it) } @@ -431,12 +488,14 @@ object NotificationUtils { private fun publicReplyAction( applicationContext: Context, notId: Int, + eventId: String, target: InlineReplyTarget, ): NotificationCompat.Action { val intent = Intent(applicationContext, NotificationReplyReceiver::class.java).apply { action = PUBLIC_REPLY_ACTION putExtra(KEY_NOTIFICATION_ID, notId) + putExtra(KEY_EVENT_ID, eventId) putExtra(KEY_ACCOUNT_NPUB, target.accountNpub) putExtra(KEY_TARGET_EVENT_ID, target.targetEventId) } @@ -446,11 +505,13 @@ object NotificationUtils { private fun markReadAction( applicationContext: Context, notId: Int, + eventId: String, ): NotificationCompat.Action { val markReadIntent = Intent(applicationContext, NotificationReplyReceiver::class.java).apply { action = MARK_READ_ACTION putExtra(KEY_NOTIFICATION_ID, notId) + putExtra(KEY_EVENT_ID, eventId) } val markReadPendingIntent = PendingIntent.getBroadcast( @@ -549,16 +610,33 @@ object NotificationUtils { // Group summaries, dedup, dismissal // --------------------------------------------------------------------- + /** + * Posts (or refreshes) our own summary for [groupKey]. + * + * The summary goes up with the **first** child, not once a second one shows up. + * Android 16 counts a group child whose summary is missing as ungrouped + * (`GroupHelper.isGroupChildWithoutSummary`) and force-groups it into the + * package's per-section aggregate bundle, next to every other ungrouped + * notification in the same shade section. The bar is low: `config_autoGroupAtCount` + * is 2, so a single summary-less child plus one other ungrouped notification is a + * bundle. The always-on relay service is exactly that other notification — ongoing + * and IMPORTANCE_LOW, it shares the Silent section with our two IMPORTANCE_LOW + * kinds (reactions and reposts), so one lone repost would end up bundled with it. + * The bundle then refuses to swipe away, because the system's aggregate summary + * inherits FLAG_ONGOING_EVENT from any child carrying it — and the service + * notification always does. + * + * Providing the summary from the start keeps the group ours and the system leaves + * it alone. It costs nothing visually: the shade hides any group with fewer than + * two children and shows the child on its own. + */ private fun NotificationManager.sendGroupSummary( category: NotificationCategory, groupKey: String, summaryId: Int, + time: Long, applicationContext: Context, ) { - val activeCount = activeNotifications.count { it.notification.group == groupKey && it.id != summaryId } - - if (activeCount < 2) return - val summaryBuilder = NotificationCompat .Builder(applicationContext, category.channelId(applicationContext)) @@ -566,8 +644,16 @@ object NotificationUtils { .setColor(category.color) .setGroup(groupKey) .setGroupSummary(true) + // The children do the alerting. Without this the summary would buzz on + // its own the moment it starts going up alongside the first child. + .setGroupAlertBehavior(NotificationCompat.GROUP_ALERT_CHILDREN) .setAutoCancel(true) .setOnlyAlertOnce(true) + // Pinned to the child's event time rather than left to default to "now". + // The summary is re-posted on every one of the enrichment path's re-renders, + // and a fresh timestamp each time would keep re-sorting the group in the + // shade while the user is looking at it. + .setWhen(time * 1000) .setStyle( NotificationCompat .InboxStyle() @@ -604,17 +690,53 @@ object NotificationUtils { // items), so bail out before touching anything when nothing is posted for it. if (activeNotifications.none { it.id == notId }) return - cancel(notId) - cancelChildlessGroupSummaries() + cancelAndPrune(notId) } - private fun NotificationManager.cancelChildlessGroupSummaries() { + /** + * Cancels [notId] and drops the group summary it leaves behind, if it was the last + * child. Use this instead of a bare [NotificationManager.cancel] for anything we + * posted through [postStandard] / [postConversation] — every one of those is a + * group child with a summary above it. + */ + fun NotificationManager.cancelAndPrune(notId: Int) { + cancel(notId) + cancelChildlessGroupSummaries(alreadyGone = notId) + } + + /** + * Drops our summaries that no longer have any children. + * + * [alreadyGone] is the id of a notification cancelled moments ago: both + * [NotificationManager.cancel] and [NotificationManager.notify] are asynchronous, + * so [NotificationManager.activeNotifications] can still be listing it and would + * otherwise keep its summary alive forever. + * + * Only summaries under [OWN_GROUP_PREFIX] are touched. The system's own aggregate + * summaries also carry FLAG_GROUP_SUMMARY and show up in this list; cancelling one + * only makes the platform rebuild it. + */ + fun NotificationManager.cancelChildlessGroupSummaries(alreadyGone: Int? = null) { val active: Array = activeNotifications + + // Collect the groups that still have a child in one pass, then cancel the + // summaries not in that set. Every child now ships with a summary, so this list + // is about twice as long as it used to be and the pairwise scan it replaces grew + // four-fold. Membership is decided by the summary flag rather than by comparing + // ids, which is also what makes it correct when a child's id happens to equal the + // summary's. + val groupsWithChildren = HashSet(active.size) + for (child in active) { + if (child.notification.flags and Notification.FLAG_GROUP_SUMMARY != 0) continue + if (child.id == alreadyGone) continue + child.notification.group?.let { groupsWithChildren.add(it) } + } + for (summary in active) { if (summary.notification.flags and Notification.FLAG_GROUP_SUMMARY == 0) continue val group = summary.notification.group ?: continue - val hasChildren = active.any { it.id != summary.id && it.notification.group == group } - if (!hasChildren) cancel(summary.id) + if (!group.startsWith(OWN_GROUP_PREFIX)) continue + if (group !in groupsWithChildren) cancel(summary.id) } }