Fix room id parsing from notification tags

Thread notification tags have the form "roomId|threadId". clearReadRoomsNotifications
was building a RoomId from the whole tag, which is only validated in debug builds, so
in release an invalid room id reached RoomListService.room(). Add
NotificationCreator.getMessageNotificationRoomId(), the inverse of messageTag(), which
only keeps the part before '|' and checks it is a valid room id.

Also make RoomListServiceInterface.roomOrNull take a RoomId instead of a String.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Benoit Marty
2026-10-15 12:40:26 +02:00
parent e1b9ba21dc
commit 9ac71fd35d
9 changed files with 106 additions and 17 deletions
@@ -12,7 +12,6 @@ import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
import dev.zacsweers.metro.SingleIn
import io.element.android.features.lockscreen.api.LockScreenService
import io.element.android.libraries.core.data.tryOrNull
import io.element.android.libraries.di.annotations.AppCoroutineScope
import io.element.android.libraries.matrix.api.MatrixClientProvider
import io.element.android.libraries.matrix.api.core.EventId
@@ -191,7 +190,7 @@ class DefaultNotificationDrawerManager(
override fun clearReadRoomsNotifications(sessionId: SessionId) {
val notifications = activeNotificationsProvider.getNotificationsForSession(sessionId)
val notificationsGroupedByRoomId = notifications.mapNotNull {
tryOrNull { RoomId(it.tag) }?.let { roomId -> roomId to it }
it.tag?.let(NotificationCreator::getMessageNotificationRoomId)?.let { roomId -> roomId to it }
}
if (notificationsGroupedByRoomId.isEmpty()) return
@@ -24,6 +24,7 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarData
import io.element.android.libraries.designsystem.components.avatar.AvatarSize
import io.element.android.libraries.designsystem.utils.CommonDrawables
import io.element.android.libraries.di.annotations.ApplicationContext
import io.element.android.libraries.matrix.api.core.MatrixPatterns
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.ThreadId
import io.element.android.libraries.matrix.api.timeline.item.event.EventType
@@ -102,11 +103,25 @@ interface NotificationCreator {
/**
* Creates a tag for a message notification given its [roomId] and optional [threadId].
*/
fun messageTag(roomId: RoomId, threadId: ThreadId?): String = if (threadId != null) {
"$roomId|$threadId"
} else {
roomId.value
fun messageTag(roomId: RoomId, threadId: ThreadId?): String = buildString {
append(roomId)
threadId?.let {
append(SEPARATOR)
append(it)
}
}
/**
* Extracts the [RoomId] from a message notification [tag] created by [messageTag].
* Returns null if the tag does not start with a valid room id.
*/
fun getMessageNotificationRoomId(tag: String): RoomId? {
return tag.substringBefore(SEPARATOR)
.takeIf(MatrixPatterns::isRoomId)
?.let(::RoomId)
}
private const val SEPARATOR = "|"
}
}
@@ -636,6 +636,43 @@ class DefaultNotificationDrawerManagerTest {
cancelNotificationResult.assertions().isNeverCalled()
}
@Test
fun `clearReadRoomsNotifications cancels thread notifications of rooms without unread counts`() = runTest {
val cancelNotificationResult = lambdaRecorder<String?, Int, Unit> { _, _ -> }
val threadTag = "$A_ROOM_ID|$A_THREAD_ID"
val sut = createClearReadRoomsManager(
cancelNotificationResult = cancelNotificationResult,
notifications = listOf(
mockk<StatusBarNotification> {
every { id } returns 1
every { tag } returns threadTag
},
),
roomInfo = aRoomInfo(id = A_ROOM_ID),
)
sut.clearReadRoomsNotifications(A_SESSION_ID)
runCurrent()
cancelNotificationResult.assertions().isCalledOnce().with(value(threadTag), value(1))
}
@Test
fun `clearReadRoomsNotifications ignores notifications whose tag is not a room id`() = runTest {
val cancelNotificationResult = lambdaRecorder<String?, Int, Unit> { _, _ -> }
val sut = createClearReadRoomsManager(
cancelNotificationResult = cancelNotificationResult,
notifications = listOf(
mockk<StatusBarNotification> {
every { id } returns 1
every { tag } returns AN_EVENT_ID.value
},
),
roomInfo = aRoomInfo(id = A_ROOM_ID),
)
sut.clearReadRoomsNotifications(A_SESSION_ID)
runCurrent()
cancelNotificationResult.assertions().isNeverCalled()
}
@Test
fun `clearReadRoomsNotifications clears the summary when it is the only notification left`() = runTest {
val cancelNotificationResult = lambdaRecorder<String?, Int, Unit> { _, _ -> }
@@ -681,7 +718,7 @@ class DefaultNotificationDrawerManagerTest {
count: Int = notifications.size,
): DefaultNotificationDrawerManager {
val matrixClient = FakeMatrixClient().apply {
getRoomInfoLambda = { Result.success(roomInfo) }
getRoomInfoLambda = { roomId -> Result.success(roomInfo?.takeIf { it.id == roomId }) }
}
return createDefaultNotificationDrawerManager(
notificationDisplayer = FakeNotificationDisplayer(cancelNotificationResult = cancelNotificationResult),
@@ -0,0 +1,36 @@
/*
* Copyright (c) 2026 Element Creations Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.push.impl.notifications.factories
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.matrix.test.AN_EVENT_ID
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.A_THREAD_ID
import org.junit.Test
class NotificationCreatorTest {
@Test
fun `getMessageNotificationRoomId returns the room id of a room message tag`() {
val tag = NotificationCreator.messageTag(A_ROOM_ID, null)
assertThat(tag).isEqualTo(A_ROOM_ID.value)
assertThat(NotificationCreator.getMessageNotificationRoomId(tag)).isEqualTo(A_ROOM_ID)
}
@Test
fun `getMessageNotificationRoomId returns the room id of a thread message tag`() {
val tag = NotificationCreator.messageTag(A_ROOM_ID, A_THREAD_ID)
assertThat(tag).isEqualTo("${A_ROOM_ID.value}|${A_THREAD_ID.value}")
assertThat(NotificationCreator.getMessageNotificationRoomId(tag)).isEqualTo(A_ROOM_ID)
}
@Test
fun `getMessageNotificationRoomId returns null for a tag which is not a room id`() {
assertThat(NotificationCreator.getMessageNotificationRoomId(AN_EVENT_ID.value)).isNull()
assertThat(NotificationCreator.getMessageNotificationRoomId("")).isNull()
}
}