From 1fe97a6af54bc0b540b5e0e76bb232588b37ab1d Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 21 Sep 2026 22:55:06 +0200 Subject: [PATCH] fix(conversations): keep conversations that arrive during a list sync A conversation list sync reads the locally known conversations after the server has answered, and treats every one the response does not mention as a conversation the user left. A conversation that reached the database while the request was in flight - a single-room fetch, a room joined on another device, one the user just created - was never in that response and could not have been, so it was deleted, taking its cached messages and chat blocks with it through the foreign key cascade. Take the known conversation ids before the request goes out and reconcile removals only against those. Anything that appeared afterwards is left alone until the next full sync, which is the first one whose response can speak about it at all. Reading the conversations themselves earlier would not do: that same read feeds the pending local state guard, which needs the freshest state to protect a favourite or read marker that is still on its way to the server. Only the removal reconcile wants the older snapshot, so only it gets one, as an id query that does not load the rows. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../OfflineFirstConversationsRepository.kt | 6 +++ .../data/database/dao/ConversationsDao.kt | 4 ++ .../utils/preview/ComposePreviewUtilsDaos.kt | 2 + ...OfflineFirstConversationsRepositoryTest.kt | 38 ++++++++++++++++--- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt index 328685638df..3b4d3f42333 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt @@ -172,6 +172,11 @@ class OfflineFirstConversationsRepository @Inject constructor( val includeStatus = isUserStatusAvailable(user) try { + // Taken before the request: the response describes the server as it was at this moment, + // so a conversation that arrives locally while the request is in flight is not one the + // response can be read as having dropped. + val knownBeforeRequest = dao.getConversationIdsForUser(user.id!!).toSet() + val conversationsList = withRetry( retries = NETWORK_FETCH_RETRIES, initialDelayMillis = NETWORK_FETCH_RETRY_INITIAL_DELAY_MS, @@ -197,6 +202,7 @@ class OfflineFirstConversationsRepository @Inject constructor( conversationsFromSync ), conversationIdsToDelete = determineLeftConversationIds(previousConversations, conversationsFromSync) + .filter { it in knownBeforeRequest } ) val roomsWithNewMessages = getRoomsWithNewMessages(conversationsFromSync, previousConversations) diff --git a/app/src/main/java/com/nextcloud/talk/data/database/dao/ConversationsDao.kt b/app/src/main/java/com/nextcloud/talk/data/database/dao/ConversationsDao.kt index 1008111556d..e590e5a2f44 100644 --- a/app/src/main/java/com/nextcloud/talk/data/database/dao/ConversationsDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/database/dao/ConversationsDao.kt @@ -25,6 +25,10 @@ interface ConversationsDao { @Query("SELECT * FROM Conversations where accountId = :accountId AND token = :token") fun getConversationForUser(accountId: Long, token: String): Flow + /** The internal ids of [accountId]'s conversations, without loading the conversations themselves. */ + @Query("SELECT internalId FROM Conversations where accountId = :accountId") + suspend fun getConversationIdsForUser(accountId: Long): List + /** * Applies a full room list sync atomically: left conversations are deleted and the server * items are upserted in one transaction, so observers of the conversations table see a single diff --git a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt index 6e3ad48553b..6cb9a729835 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt @@ -258,6 +258,8 @@ class DummyUserDaoImpl : UsersDao() { } class DummyConversationDaoImpl : ConversationsDao { + override suspend fun getConversationIdsForUser(accountId: Long): List = emptyList() + override fun getConversationsForUser(accountId: Long): Flow> = flowOf() override fun getConversationForUser(accountId: Long, token: String): Flow = flowOf() diff --git a/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt b/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt index cb413f7d31f..e3202b87c6d 100644 --- a/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt +++ b/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepositoryTest.kt @@ -92,7 +92,7 @@ class OfflineFirstConversationsRepositoryTest { whenever(connectivityManager.restrictBackgroundStatus) .thenReturn(ConnectivityManager.RESTRICT_BACKGROUND_STATUS_DISABLED) - whenever(dao.getConversationsForUser(ACCOUNT_ID)).thenReturn(flowOf(emptyList())) + stubStoredConversations() whenever(conversationListUpdater.preservePendingLocalState(any(), any())) .thenAnswer { invocation -> invocation.getArgument>(1) } @@ -140,7 +140,7 @@ class OfflineFirstConversationsRepositoryTest { fun `getRooms skips deleting local conversations when the server returns an empty list`() = runBlocking { val previous = conversation(token = ROOM_TOKEN, lastActivity = 5, unreadMessages = 0).asEntity(ACCOUNT_ID) - whenever(dao.getConversationsForUser(ACCOUNT_ID)).thenReturn(flowOf(listOf(previous))) + stubStoredConversations(previous) whenever(network.getRooms(any(), any(), any())).thenReturn(Observable.just(emptyList())) repository.getRooms(user()).join() @@ -155,7 +155,7 @@ class OfflineFirstConversationsRepositoryTest { runBlocking { val staying = conversation(token = "roomA", lastActivity = 5, unreadMessages = 0).asEntity(ACCOUNT_ID) val leaving = conversation(token = "roomB", lastActivity = 5, unreadMessages = 0).asEntity(ACCOUNT_ID) - whenever(dao.getConversationsForUser(ACCOUNT_ID)).thenReturn(flowOf(listOf(staying, leaving))) + stubStoredConversations(staying, leaving) val stayingRoom = conversation(token = "roomA", lastActivity = 5, unreadMessages = 0) whenever(network.getRooms(any(), any(), any())).thenReturn(Observable.just(listOf(stayingRoom))) @@ -170,7 +170,7 @@ class OfflineFirstConversationsRepositoryTest { fun `getRooms merges the server response through the conversation list updater`() = runBlocking { val previous = conversation(token = ROOM_TOKEN, lastActivity = 5, unreadMessages = 0).asEntity(ACCOUNT_ID) - whenever(dao.getConversationsForUser(ACCOUNT_ID)).thenReturn(flowOf(listOf(previous))) + stubStoredConversations(previous) val serverRoom = conversation(token = ROOM_TOKEN, lastActivity = 6, unreadMessages = 1) whenever(network.getRooms(any(), any(), any())).thenReturn(Observable.just(listOf(serverRoom))) @@ -250,8 +250,7 @@ class OfflineFirstConversationsRepositoryTest { conversation(token = "unchanged", lastActivity = 10, unreadMessages = 0).asEntity(ACCOUNT_ID) val previousNoBlock = conversation(token = "noBlockUnread", lastActivity = 10, unreadMessages = 3).asEntity(ACCOUNT_ID) - whenever(dao.getConversationsForUser(ACCOUNT_ID)) - .thenReturn(flowOf(listOf(previousUnchanged, previousNoBlock))) + stubStoredConversations(previousUnchanged, previousNoBlock) val unchangedRoom = conversation(token = "unchanged", lastActivity = 10, unreadMessages = 0) val noBlockRoom = conversation(token = "noBlockUnread", lastActivity = 10, unreadMessages = 3) @@ -442,6 +441,33 @@ class OfflineFirstConversationsRepositoryTest { lastReadMessage = lastReadMessage ) + @Test + fun `a conversation that arrives while the request is in flight is not treated as left`() = + runBlocking { + val known = conversation(token = "known", lastActivity = 5, unreadMessages = 0).asEntity(ACCOUNT_ID) + val arrivedDuringRequest = + conversation(token = "arrived", lastActivity = 5, unreadMessages = 0).asEntity(ACCOUNT_ID) + // the ids were taken before the request; the rows are read after it, by which time the + // second conversation had been written by something else + wheneverBlocking { dao.getConversationIdsForUser(ACCOUNT_ID) }.thenReturn(listOf(known.internalId)) + whenever(dao.getConversationsForUser(ACCOUNT_ID)) + .thenReturn(flowOf(listOf(known, arrivedDuringRequest))) + whenever(network.getRooms(any(), any(), any())).thenReturn(Observable.just(listOf())) + + repository.getRooms(user()).join() + + verifyBlocking(dao) { syncConversationsForUser(eq(ACCOUNT_ID), any(), eq(listOf(known.internalId))) } + } + + /** + * Stubs both reads the sync makes of the conversations table: the rows it merges against, and + * the ids it takes before the request to know which conversations the response can speak about. + */ + private fun stubStoredConversations(vararg stored: ConversationEntity) { + whenever(dao.getConversationsForUser(ACCOUNT_ID)).thenReturn(flowOf(stored.toList())) + wheneverBlocking { dao.getConversationIdsForUser(ACCOUNT_ID) }.thenReturn(stored.map { it.internalId }) + } + companion object { private const val ACCOUNT_ID = 1L private const val BASE_URL = "https://server.example.com"