Don't delete conversations that arrive while a list sync is in flight

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)

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<ConversationEntity?>
 
+    /** 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<String>
+
     /**
      * 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

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<String> = emptyList()
+
     override fun getConversationsForUser(accountId: Long): Flow<List<ConversationEntity>> = flowOf()
 
     override fun getConversationForUser(accountId: Long, token: String): Flow<ConversationEntity?> = flowOf()

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<List<ConversationEntity>>(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"