From 4e3853a7188fa53382dee4590f45169e3f115e8c Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Mon, 18 May 2026 22:18:52 +0200 Subject: [PATCH] Attempt to fix room list item duplicates at midnight (#6793) * Attempt to fix room list item duplicates at midnight This seems to happen because of a race condition between `RoomListDataSource.observeDateTimeChanges` and `RoomListDataSource.replaceWith` being called at almost the same time and the first one using the newly received items from observing the timeline items but not updating the cache which will be later reused by `replaceWith`, containing incorrect indices --- .../impl/datasource/RoomListDataSource.kt | 5 +- .../impl/datasource/RoomListDataSourceTest.kt | 63 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/features/home/impl/src/main/kotlin/io/element/android/features/home/impl/datasource/RoomListDataSource.kt b/features/home/impl/src/main/kotlin/io/element/android/features/home/impl/datasource/RoomListDataSource.kt index 3ff4339bb2..56b2c1ade4 100644 --- a/features/home/impl/src/main/kotlin/io/element/android/features/home/impl/datasource/RoomListDataSource.kt +++ b/features/home/impl/src/main/kotlin/io/element/android/features/home/impl/datasource/RoomListDataSource.kt @@ -83,8 +83,8 @@ class RoomListDataSource( val loadingState = roomList.loadingState - fun launchIn(coroutineScope: CoroutineScope) { - roomList + fun launchIn(coroutineScope: CoroutineScope): Job { + return roomList .summaries .onEach { roomSummaries -> replaceWith(roomSummaries) @@ -212,6 +212,7 @@ class RoomListDataSource( private suspend fun rebuildAllRoomSummaries() { lock.withLock { roomList.summaries.replayCache.firstOrNull()?.let { roomSummaries -> + diffCacheUpdater.updateWith(roomSummaries) buildAndEmitAllRooms(roomSummaries, useCache = false) } } diff --git a/features/home/impl/src/test/kotlin/io/element/android/features/home/impl/datasource/RoomListDataSourceTest.kt b/features/home/impl/src/test/kotlin/io/element/android/features/home/impl/datasource/RoomListDataSourceTest.kt index ace1ef2eaa..6df5e05df5 100644 --- a/features/home/impl/src/test/kotlin/io/element/android/features/home/impl/datasource/RoomListDataSourceTest.kt +++ b/features/home/impl/src/test/kotlin/io/element/android/features/home/impl/datasource/RoomListDataSourceTest.kt @@ -16,6 +16,7 @@ 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.A_ROOM_ID_3 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 @@ -197,6 +198,68 @@ class RoomListDataSourceTest { } } + @Test + fun `regression test for race with DateTimeObserver and new items`() = runTest { + val roomList = FakeDynamicRoomList(summaries = MutableStateFlow(listOf(aRoomSummary(), aRoomSummary(A_ROOM_ID_2)))) + val roomListService = FakeRoomListService( + createRoomListLambda = { roomList } + ).apply { + postState(RoomListService.State.Running) + } + val dateTimeObserver = FakeDateTimeObserver() + var dateFormatterResult = "Today" + val dateFormatter = FakeDateFormatter({ _, _, _ -> dateFormatterResult }) + val roomListDataSource = createRoomListDataSource( + roomListService = roomListService, + roomListRoomSummaryFactory = aRoomListRoomSummaryFactory( + dateFormatter = dateFormatter, + ), + dateTimeObserver = dateTimeObserver, + ) + roomListDataSource.roomSummariesFlow.test { + // Observe room list items changes + val job = roomListDataSource.launchIn(backgroundScope) + // Get the initial room list + val initialRoomList = awaitItem() + assertThat(initialRoomList).hasSize(2) + assertThat(initialRoomList[0].roomId).isEqualTo(A_ROOM_ID) + assertThat(initialRoomList[0].timestamp).isEqualTo(dateFormatterResult) + assertThat(initialRoomList[1].roomId).isEqualTo(A_ROOM_ID_2) + assertThat(initialRoomList[1].timestamp).isEqualTo(dateFormatterResult) + + // Stop processing room list updates so we can force a race condition with the date time observer updates + job.cancel() + + // Trigger a date change and a new item at the same time + dateFormatterResult = "Yesterday" + roomList.summaries.tryEmit(listOf(aRoomSummary(roomId = A_ROOM_ID), aRoomSummary(roomId = A_ROOM_ID_3), aRoomSummary(roomId = A_ROOM_ID_2))) + dateTimeObserver.given(DateTimeObserver.Event.DateChanged(Instant.MIN, Instant.now())) + + // The race condition would have caused the cache indices to be corrupted and only 2 items would be emitted + val rebuiltRoomList = awaitItem() + assertThat(rebuiltRoomList).hasSize(3) + assertThat(rebuiltRoomList[0].roomId).isEqualTo(A_ROOM_ID) + assertThat(rebuiltRoomList[0].timestamp).isEqualTo(dateFormatterResult) + assertThat(rebuiltRoomList[1].roomId).isEqualTo(A_ROOM_ID_3) + assertThat(rebuiltRoomList[1].timestamp).isEqualTo(dateFormatterResult) + assertThat(rebuiltRoomList[2].roomId).isEqualTo(A_ROOM_ID_2) + assertThat(rebuiltRoomList[2].timestamp).isEqualTo(dateFormatterResult) + + // Restart processing room list updates + roomListDataSource.launchIn(backgroundScope) + + // Check there is a new list and it's not the same as the previous one + val newRoomList = awaitItem() + assertThat(newRoomList).hasSize(3) + assertThat(newRoomList[0].roomId).isEqualTo(A_ROOM_ID) + assertThat(newRoomList[0].timestamp).isEqualTo(dateFormatterResult) + assertThat(newRoomList[1].roomId).isEqualTo(A_ROOM_ID_3) + assertThat(newRoomList[1].timestamp).isEqualTo(dateFormatterResult) + assertThat(newRoomList[2].roomId).isEqualTo(A_ROOM_ID_2) + assertThat(newRoomList[2].timestamp).isEqualTo(dateFormatterResult) + } + } + private fun TestScope.createRoomListDataSource( roomListService: FakeRoomListService = FakeRoomListService(), roomListRoomSummaryFactory: RoomListRoomSummaryFactory = aRoomListRoomSummaryFactory(),