From d94e69be6feed5b83663eab92028422f3a862861 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 | 3 ++ .../data/database/dao/ConversationsDao.kt | 4 +++ .../utils/preview/ComposePreviewUtilsDaos.kt | 2 ++ ...OfflineFirstConversationsRepositoryTest.kt | 36 +++++++++++++++---- 4 files changed, 39 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 91738cb1a9b..d8d62e9e914 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 @@ -174,6 +174,8 @@ class OfflineFirstConversationsRepository @Inject constructor( val includeStatus = isUserStatusAvailable(user) try { + val knownBeforeRequest = dao.getConversationIdsForUser(user.id!!).toSet() + val conversationsList = withRetry( retries = NETWORK_FETCH_RETRIES, initialDelayMillis = NETWORK_FETCH_RETRY_INITIAL_DELAY_MS, @@ -199,6 +201,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 1dc27b7788a..3fb794f8e54 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 @@ -240,6 +240,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 73556891b88..294e4ad9788 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 @@ -93,7 +93,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) } @@ -142,7 +142,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() @@ -157,7 +157,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))) @@ -172,7 +172,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))) @@ -252,8 +252,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) @@ -444,6 +443,31 @@ 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) + 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"