Fix room list duplicate-detection telemetry crashing before it can report (#6791)

* Add room dupe regression tests

* Fix telemetry path for dedupe discovery

features/home/impl/src/test/kotlin/io/element/android/features/home/impl/datasource/RoomListDataSourceTest.kt

@@ -14,6 +14,8 @@ import io.element.android.features.home.impl.FakeDateTimeObserver
 import io.element.android.libraries.androidutils.system.DateTimeObserver
 import io.element.android.libraries.dateformatter.test.FakeDateFormatter
 import io.element.android.libraries.matrix.api.roomlist.RoomListService
+import io.element.android.libraries.matrix.test.A_ROOM_ID
+import io.element.android.libraries.matrix.test.A_ROOM_ID_2
 import io.element.android.libraries.matrix.test.notificationsettings.FakeNotificationSettingsService
 import io.element.android.libraries.matrix.test.room.aRoomSummary
 import io.element.android.libraries.matrix.test.roomlist.FakeDynamicRoomList
@@ -100,11 +102,107 @@ class RoomListDataSourceTest {
         }
     }
 
+    /**
+     * Tracking issue #4182: rooms duplicated in the room list around midnight.
+     *
+     * If the SDK ever leaks a list containing the same roomId twice (the suspected cause of #4182),
+     * the UI mapper's `distinctBy` safety net in [RoomListDataSource.buildAndEmitAllRooms] must
+     * remove the duplicate AND `analyticsService.trackError` must fire so the team can root-cause
+     * it via Sentry.
+     */
+    @Test
+    fun `when SDK summaries source contains duplicate roomIds, UI layer dedupes and reports trackError`() = runTest {
+        val analyticsService = FakeAnalyticsService()
+        val duplicatedSummaries = listOf(
+            aRoomSummary(roomId = A_ROOM_ID),
+            aRoomSummary(roomId = A_ROOM_ID),
+            aRoomSummary(roomId = A_ROOM_ID_2),
+        )
+        val roomList = FakeDynamicRoomList(summaries = MutableStateFlow(duplicatedSummaries))
+        val roomListService = FakeRoomListService(
+            createRoomListLambda = { roomList }
+        ).apply {
+            postState(RoomListService.State.Running)
+        }
+        val roomListDataSource = createRoomListDataSource(
+            roomListService = roomListService,
+            analyticsService = analyticsService,
+        )
+
+        roomListDataSource.roomSummariesFlow.test {
+            roomListDataSource.launchIn(backgroundScope)
+            val list = awaitItem()
+            assertThat(list.map { it.roomId }).containsExactly(A_ROOM_ID, A_ROOM_ID_2).inOrder()
+            assertThat(analyticsService.trackedErrors).hasSize(1)
+        }
+    }
+
+    /**
+     * Tracking issue #4182.
+     *
+     * Targeted scenario: a `DateChanged` tick fires after an initial SDK emit, then a follow-up
+     * SDK emit lands (mimicking "midnight, then a new message arrives"). Even though the diffCache
+     * is bypassed during the rebuild (`useCache = false`), the final state must contain each
+     * roomId exactly once and trackError must not fire on a happy path.
+     */
+    @Test
+    fun `interleaved date change and SDK update with overlapping content does not produce duplicates`() = runTest {
+        val analyticsService = FakeAnalyticsService()
+        val summariesFlow = MutableStateFlow(
+            listOf(
+                aRoomSummary(roomId = A_ROOM_ID),
+                aRoomSummary(roomId = A_ROOM_ID_2),
+            )
+        )
+        val roomList = FakeDynamicRoomList(summaries = summariesFlow)
+        val roomListService = FakeRoomListService(
+            createRoomListLambda = { roomList }
+        ).apply {
+            postState(RoomListService.State.Running)
+        }
+        val dateTimeObserver = FakeDateTimeObserver()
+        val roomListDataSource = createRoomListDataSource(
+            roomListService = roomListService,
+            dateTimeObserver = dateTimeObserver,
+            analyticsService = analyticsService,
+        )
+
+        roomListDataSource.roomSummariesFlow.test {
+            roomListDataSource.launchIn(backgroundScope)
+            val initial = awaitItem()
+            assertThat(initial.map { it.roomId }).containsExactly(A_ROOM_ID, A_ROOM_ID_2).inOrder()
+
+            // Midnight ticks while the cache holds [A_ROOM_ID, A_ROOM_ID_2]
+            dateTimeObserver.given(DateTimeObserver.Event.DateChanged(Instant.MIN, Instant.now()))
+            val afterMidnight = awaitItem()
+            assertThat(afterMidnight.map { it.roomId }).containsExactly(A_ROOM_ID, A_ROOM_ID_2).inOrder()
+
+            // A new message bumps A_ROOM_ID — different unread count makes the StateFlow see this
+            // as a new value
+            summariesFlow.value = listOf(
+                aRoomSummary(roomId = A_ROOM_ID, numUnreadMessages = 1),
+                aRoomSummary(roomId = A_ROOM_ID_2),
+            )
+            val afterMessage = awaitItem()
+            assertThat(afterMessage.map { it.roomId }).containsExactly(A_ROOM_ID, A_ROOM_ID_2).inOrder()
+            assertThat(afterMessage.map { it.roomId }.toSet()).hasSize(afterMessage.size)
+
+            // Second midnight rebuild after the new message
+            dateTimeObserver.given(DateTimeObserver.Event.DateChanged(Instant.MIN, Instant.now()))
+            val afterSecondMidnight = awaitItem()
+            assertThat(afterSecondMidnight.map { it.roomId }).containsExactly(A_ROOM_ID, A_ROOM_ID_2).inOrder()
+            assertThat(afterSecondMidnight.map { it.roomId }.toSet()).hasSize(afterSecondMidnight.size)
+
+            assertThat(analyticsService.trackedErrors).isEmpty()
+        }
+    }
+
     private fun TestScope.createRoomListDataSource(
         roomListService: FakeRoomListService = FakeRoomListService(),
         roomListRoomSummaryFactory: RoomListRoomSummaryFactory = aRoomListRoomSummaryFactory(),
         notificationSettingsService: FakeNotificationSettingsService = FakeNotificationSettingsService(),
         dateTimeObserver: FakeDateTimeObserver = FakeDateTimeObserver(),
+        analyticsService: FakeAnalyticsService = FakeAnalyticsService(),
     ) = RoomListDataSource(
         roomListService = roomListService,
         roomListRoomSummaryFactory = roomListRoomSummaryFactory,
@@ -112,6 +210,6 @@ class RoomListDataSourceTest {
         notificationSettingsService = notificationSettingsService,
         sessionCoroutineScope = backgroundScope,
         dateTimeObserver = dateTimeObserver,
-        analyticsService = FakeAnalyticsService(),
+        analyticsService = analyticsService,
     )
 }

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomListEntriesUpdateExt.kt

@@ -16,25 +16,25 @@ import org.matrix.rustcomponents.sdk.RoomListEntriesUpdate
 internal fun RoomListEntriesUpdate.describe(): String {
     return when (this) {
         is RoomListEntriesUpdate.Set -> {
-            "Set #$index to '${value.displayName()}'"
+            "Set #$index to '${value.id()}'"
         }
         is RoomListEntriesUpdate.Append -> {
-            "Append ${values.map { "'" + it.displayName() + "'" }}"
+            "Append ${values.map { "'" + it.id() + "'" }}"
         }
         is RoomListEntriesUpdate.PushBack -> {
-            "PushBack '${value.displayName()}'"
+            "PushBack '${value.id()}'"
         }
         is RoomListEntriesUpdate.PushFront -> {
-            "PushFront '${value.displayName()}'"
+            "PushFront '${value.id()}'"
         }
         is RoomListEntriesUpdate.Insert -> {
-            "Insert at #$index: '${value.displayName()}'"
+            "Insert at #$index: '${value.id()}'"
         }
         is RoomListEntriesUpdate.Remove -> {
             "Remove #$index"
         }
         is RoomListEntriesUpdate.Reset -> {
-            "Reset all to ${values.map { "'" + it.displayName() + "'" }}"
+            "Reset all to ${values.map { "'" + it.id() + "'" }}"
         }
         RoomListEntriesUpdate.PopBack -> {
             "PopBack"

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessor.kt

@@ -112,6 +112,11 @@ class RoomSummaryListProcessor(
     private suspend fun updateRoomSummaries(updates: List<RoomListEntriesUpdate>, block: suspend MutableList<RoomSummary>.() -> Unit) = withContext(
         coroutineContext
     ) {
+        // Capture the description before applying updates: applyUpdate consumes each Room via
+        // `entry.use { ... }` which destroys it, and the duplicate-detection branch below reads
+        // id() through `describe()`. Without this capture the trackError call crashes before it
+        // can be reported.
+        val updatesDescription = updates.description()
         mutex.withLock {
             val current = roomSummaries.replayCache.lastOrNull()
             val mutableRoomSummaries = current.orEmpty().toMutableList()
@@ -126,7 +131,7 @@ class RoomSummaryListProcessor(
                     analyticsService.trackError(
                         IllegalStateException(
                             "Found duplicates in room summaries after a list update from the SDK: $duplicates. " +
-                                "Updates: ${updates.description()}"
+                                "Updates: $updatesDescription"
                         )
                     )
                 }

libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessorTest.kt

@@ -173,16 +173,68 @@ class RoomSummaryListProcessorTest {
         assertThat(summaries.value[index].roomId).isEqualTo(A_ROOM_ID_3)
     }
 
+    /**
+     * Tracking issue #4182 / #5031: rooms duplicated in the room list.
+     *
+     * If duplicates are present in the upstream summaries flow, the dedupe safety net in
+     * [RoomSummaryListProcessor.updateRoomSummaries] must remove them and report the incident via
+     * [analyticsService.trackError]. Uses an empty update to drive the dedupe path without
+     * passing a Rust Room through the destroy-on-use path.
+     */
+    @Test
+    fun `pre-existing duplicates in summaries are deduped on next update and trackError fires`() = runTest {
+        summaries.value = listOf(
+            aRoomSummary(roomId = A_ROOM_ID),
+            aRoomSummary(roomId = A_ROOM_ID), // simulated SDK-side leak
+            aRoomSummary(roomId = A_ROOM_ID_2),
+        )
+        val analyticsService = FakeAnalyticsService()
+        val processor = createProcessor(analyticsService = analyticsService)
+
+        processor.postUpdate(emptyList())
+
+        assertThat(summaries.value.map { it.roomId }).containsExactly(A_ROOM_ID, A_ROOM_ID_2).inOrder()
+        assertThat(analyticsService.trackedErrors).hasSize(1)
+    }
+
+    /**
+     * Tracking issue #4182 / #5031.
+     *
+     * Insert is the most likely Rust-SDK trigger for a duplicate-room report: it blindly inserts
+     * a new entry at an index without checking whether the roomId already exists. Before the
+     * describe-capture fix, the dedupe branch in [updateRoomSummaries] would call `Room.id()`
+     * on an already-destroyed Room (because [applyUpdate] consumes each value via
+     * `entry.use { ... }`) and crash before [trackError] could be invoked. This test guards the
+     * fix: the Insert is processed, the list is emitted deduplicated, and the tracked error
+     * message carries the human-readable description of the offending update.
+     */
+    @Test
+    fun `Insert that triggers dedupe is reported via trackError without crashing`() = runTest {
+        summaries.value = listOf(aRoomSummary(roomId = A_ROOM_ID))
+        val analyticsService = FakeAnalyticsService()
+        val processor = createProcessor(analyticsService = analyticsService)
+
+        processor.postUpdate(listOf(RoomListEntriesUpdate.Insert(0u, aRustRoom(A_ROOM_ID))))
+
+        assertThat(summaries.value.map { it.roomId }).containsExactly(A_ROOM_ID)
+        assertThat(analyticsService.trackedErrors).hasSize(1)
+        val message = analyticsService.trackedErrors.single().message.orEmpty()
+        assertThat(message).contains("Found duplicates")
+        assertThat(message).contains("Insert at #0")
+    }
+
     private fun aRustRoom(roomId: RoomId = A_ROOM_ID) = FakeFfiRoom(
         roomId = roomId,
         latestEventLambda = { LatestEventValue.None }
     )
 
-    private fun TestScope.createProcessor() = RoomSummaryListProcessor(
+    private fun TestScope.createProcessor(
+        analyticsService: FakeAnalyticsService = FakeAnalyticsService(),
+    ) = RoomSummaryListProcessor(
         summaries,
         FakeFfiRoomListService(),
         coroutineContext = StandardTestDispatcher(testScheduler),
         roomSummaryFactory = RoomSummaryFactory(),
-        analyticsService = FakeAnalyticsService(),
+        analyticsService = analyticsService,
     )
 }