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
This commit is contained in:
parent
1774d1d39a
commit
4e3853a718
2 changed files with 66 additions and 2 deletions
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue