🔄️ Improve conversation list performance

app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt

@@ -2790,20 +2790,7 @@ class ChatActivity :
     }
 
     private fun updateRemoteLastReadMessageIfNeeded() {
-        if (this::spreedCapabilities.isInitialized) {
-            spreedCapabilities?.let {
-                val url = ApiUtils.getUrlForChatReadMarker(
-                    ApiUtils.getChatApiVersion(it, intArrayOf(ApiUtils.API_V1)),
-                    conversationUser.baseUrl!!,
-                    roomToken
-                )
-
-                chatViewModel.updateRemoteLastReadMessageIfNeeded(
-                    credentials = credentials!!,
-                    url = url
-                )
-            }
-        }
+        chatViewModel.updateRemoteLastReadMessageIfNeeded()
     }
 
     private fun isActivityNotChangingConfigurations(): Boolean = !isChangingConfigurations
@@ -3487,15 +3474,7 @@ class ChatActivity :
     }
 
     private fun markAsRead(messageId: Int) {
-        chatViewModel.setChatReadMessage(
-            credentials!!,
-            ApiUtils.getUrlForChatReadMarker(
-                ApiUtils.getChatApiVersion(spreedCapabilities, intArrayOf(ApiUtils.API_V1)),
-                conversationUser?.baseUrl!!,
-                roomToken
-            ),
-            messageId
-        )
+        chatViewModel.setChatReadMessage(messageId)
     }
 
     fun markAsUnread(chatMessage: ChatMessage) {
@@ -3510,15 +3489,7 @@ class ChatActivity :
         } else {
             0
         }
-        chatViewModel.setChatReadMessage(
-            credentials!!,
-            ApiUtils.getUrlForChatReadMarker(
-                ApiUtils.getChatApiVersion(spreedCapabilities, intArrayOf(ApiUtils.API_V1)),
-                conversationUser.baseUrl!!,
-                roomToken
-            ),
-            lastReadMessage
-        )
+        chatViewModel.setChatReadMessage(lastReadMessage)
     }
 
     fun copyMessage(message: ChatMessage?) {

app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt

@@ -72,6 +72,24 @@ interface ChatMessageRepository : LifecycleAwareManager {
      */
     suspend fun fetchNewMessages(): Boolean
 
+    /**
+     * Optimistically writes the user's read state into the local conversation entry, so the
+     * conversation list reflects it immediately. Sending the read marker to the server is the
+     * caller's concern; the next room list sync re-asserts the server state either way.
+     */
+    suspend fun updateLocalReadState(lastReadMessage: Int)
+
+    /**
+     * Registers [lastReadMessage] as a pending read marker synchronously, without going through
+     * [updateLocalReadState]'s suspending database reads first. Call this before launching the
+     * actual local write: leaving the chat can race a room list sync triggered by the conversation
+     * list resuming at (almost) the same time, and that sync's response must find the marker
+     * already pending to guard against a server state it computed before the marker was sent -
+     * a gap of even a couple hundred milliseconds while registration waits on database reads is
+     * enough for that race to lose.
+     */
+    fun markPendingReadMarker(lastReadMessage: Int)
+
     /**
      * Loads messages from local storage. If the messages are not found, then it
      * synchronizes the database with the server, before retrying exactly once. Only

app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt

@@ -12,6 +12,7 @@ import android.os.SystemClock
 import android.util.Log
 import com.nextcloud.talk.chat.data.model.ChatMessage
 import com.nextcloud.talk.chat.domain.ChatPullResult
+import com.nextcloud.talk.conversationlist.data.network.ConversationListUpdater
 import com.nextcloud.talk.data.database.dao.ChatBlocksDao
 import com.nextcloud.talk.data.database.dao.ChatMessagesDao
 import com.nextcloud.talk.data.database.mappers.asEntity
@@ -43,12 +44,13 @@ import javax.inject.Inject
  * per-room coalescing bookkeeping of [catchUpRoom], which collapses bursts of catch-up requests
  * (e.g. one push notification per incoming message) into few actual fetches.
  */
-@Suppress("TooManyFunctions")
+@Suppress("TooManyFunctions", "LargeClass")
 class ChatMessageSyncer @Inject constructor(
     private val chatDao: ChatMessagesDao,
     private val chatBlocksDao: ChatBlocksDao,
     private val network: ChatNetworkDataSource,
-    private val networkMonitor: NetworkMonitor
+    private val networkMonitor: NetworkMonitor,
+    private val conversationListUpdater: ConversationListUpdater
 ) {
 
     /**
@@ -91,13 +93,18 @@ class ChatMessageSyncer @Inject constructor(
      * [syncFailed] is true when the sync ended in a transient error (offline, failed request), so
      * callers like a background worker can retry later. It stays false for skips that retrying
      * would not change, e.g. a missing server capability.
+     *
+     * [newestPersistedMessage] is the newest persisted message that qualifies as a conversation's
+     * last message (system messages that never change a conversation's preview are skipped). It
+     * feeds the conversation list update after a background catch-up.
      */
     data class SyncOutcome(
         val persistedNewMessages: Boolean,
         val newestPersistedMessageId: Long?,
         val oldestPersistedMessageId: Long? = null,
         val persistedMessageCount: Int = 0,
-        val syncFailed: Boolean = false
+        val syncFailed: Boolean = false,
+        val newestPersistedMessage: ChatMessageJson? = null
     )
 
     /**
@@ -357,6 +364,8 @@ class ChatMessageSyncer @Inject constructor(
             Log.d(TAG, "Background catch-up for room ${target.roomToken}: no new messages")
         }
 
+        conversationListUpdater.updateConversationFromCatchUp(target, outcome)
+
         return outcome
     }
 
@@ -467,7 +476,8 @@ class ChatMessageSyncer @Inject constructor(
             newestPersistedMessageId = backlogOutcome.newestPersistedMessageId ?: nextAnchor,
             oldestPersistedMessageId = anchorOutcome.oldestPersistedMessageId,
             persistedMessageCount = anchorOutcome.persistedMessageCount + backlogOutcome.persistedMessageCount,
-            syncFailed = backlogOutcome.syncFailed
+            syncFailed = backlogOutcome.syncFailed,
+            newestPersistedMessage = backlogOutcome.newestPersistedMessage ?: anchorOutcome.newestPersistedMessage
         )
     }
 
@@ -497,6 +507,7 @@ class ChatMessageSyncer @Inject constructor(
         var totalCount = 0
         var oldestPersisted: Long? = null
         var newestPersisted: Long? = null
+        var newestPersistedMessage: ChatMessageJson? = null
 
         repeat(MAX_BACKLOG_ROUNDS) {
             val fieldMap = buildFieldMap(
@@ -515,6 +526,7 @@ class ChatMessageSyncer @Inject constructor(
                 totalCount += roundOutcome.persistedMessageCount
                 oldestPersisted = oldestPersisted ?: roundOutcome.oldestPersistedMessageId
                 newestPersisted = roundOutcome.newestPersistedMessageId ?: newestPersisted
+                newestPersistedMessage = roundOutcome.newestPersistedMessage ?: newestPersistedMessage
             }
 
             val caughtUp = !roundOutcome.persistedNewMessages || roundOutcome.persistedMessageCount < limit
@@ -525,7 +537,8 @@ class ChatMessageSyncer @Inject constructor(
                     newestPersistedMessageId = newestPersisted,
                     oldestPersistedMessageId = oldestPersisted,
                     persistedMessageCount = totalCount,
-                    syncFailed = roundOutcome.syncFailed
+                    syncFailed = roundOutcome.syncFailed,
+                    newestPersistedMessage = newestPersistedMessage
                 )
             }
             anchor = nextAnchor
@@ -556,7 +569,8 @@ class ChatMessageSyncer @Inject constructor(
             newestPersistedMessageId = fallbackOutcome.newestPersistedMessageId,
             oldestPersistedMessageId = fallbackOutcome.oldestPersistedMessageId,
             persistedMessageCount = fallbackOutcome.persistedMessageCount,
-            syncFailed = fallbackOutcome.syncFailed
+            syncFailed = fallbackOutcome.syncFailed,
+            newestPersistedMessage = fallbackOutcome.newestPersistedMessage ?: newestPersistedMessage
         )
     }
 
@@ -681,11 +695,19 @@ class ChatMessageSyncer @Inject constructor(
                 events
             )
             persistedMessages.maxOfOrNull { it.id }?.let { recordHttpSyncedMessageId(target, it) }
+            val newestPersistedMessage = if (persistedMessages.isNotEmpty()) {
+                result.messages
+                    .filter { it.systemMessageType !in ConversationListUpdater.LAST_MESSAGE_HIDDEN_SYSTEM_TYPES }
+                    .maxByOrNull { it.id }
+            } else {
+                null
+            }
             SyncOutcome(
                 persistedNewMessages = persistedMessages.isNotEmpty(),
                 newestPersistedMessageId = persistedMessages.maxOfOrNull { it.id },
                 oldestPersistedMessageId = persistedMessages.minOfOrNull { it.id },
-                persistedMessageCount = persistedMessages.size
+                persistedMessageCount = persistedMessages.size,
+                newestPersistedMessage = newestPersistedMessage
             )
         } else {
             Log.d(TAG, "No new messages to update")
@@ -967,6 +989,7 @@ class ChatMessageSyncer @Inject constructor(
             SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null, syncFailed = true)
 
         private const val DEFAULT_MESSAGES_LIMIT = 100
+
         private const val MILLIS_PER_SECOND = 1000L
         private const val ROOM_REFRESH_MAX_AGE_MILLIS = 3 * 60 * 60 * 1000L // 3 hours
         private const val CATCH_UP_COOLDOWN_MILLIS = 5_000L

app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt

@@ -12,6 +12,7 @@ import android.os.Bundle
 import android.util.Log
 import com.nextcloud.talk.chat.data.ChatMessageRepository
 import com.nextcloud.talk.chat.data.model.ChatMessage
+import com.nextcloud.talk.conversationlist.data.network.ConversationListUpdater
 import com.nextcloud.talk.data.database.dao.ChatBlocksDao
 import com.nextcloud.talk.data.database.dao.ChatMessagesDao
 import com.nextcloud.talk.data.database.mappers.asEntity
@@ -56,7 +57,8 @@ class OfflineFirstChatRepository @Inject constructor(
     private val chatBlocksDao: ChatBlocksDao,
     private val network: ChatNetworkDataSource,
     private val networkMonitor: NetworkMonitor,
-    private val syncer: ChatMessageSyncer
+    private val syncer: ChatMessageSyncer,
+    private val conversationListUpdater: ConversationListUpdater
 ) : ChatMessageRepository {
 
     lateinit var currentUser: User
@@ -363,6 +365,14 @@ class OfflineFirstChatRepository @Inject constructor(
         return outcome.persistedNewMessages
     }
 
+    override suspend fun updateLocalReadState(lastReadMessage: Int) {
+        conversationListUpdater.updateLocalReadState(syncTarget, lastReadMessage)
+    }
+
+    override fun markPendingReadMarker(lastReadMessage: Int) {
+        conversationListUpdater.markPendingReadMarker(syncTarget.internalConversationId, lastReadMessage)
+    }
+
     override suspend fun loadMoreMessages(
         anchorMessageId: Long,
         direction: ChatMessageRepository.LoadMoreDirection,

app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt

@@ -40,6 +40,7 @@ import com.nextcloud.talk.data.database.mappers.toDomainModel
 import com.nextcloud.talk.data.database.model.ChatMessageEntity
 import com.nextcloud.talk.data.user.model.User
 import com.nextcloud.talk.extensions.toIntOrZero
+import com.nextcloud.talk.jobs.ReadMarkerSyncWorker
 import com.nextcloud.talk.jobs.ShareOperationWorker
 import com.nextcloud.talk.jobs.UploadAndShareFilesWorker
 import com.nextcloud.talk.logger.Logger
@@ -1799,45 +1800,48 @@ class ChatViewModel @AssistedInject constructor(
     /**
      * Please use with caution to not spam the server
      */
-    fun updateRemoteLastReadMessageIfNeeded(credentials: String, url: String) {
+    fun updateRemoteLastReadMessageIfNeeded() {
         Log.d(TAG, "updateRemoteLastReadMessageIfNeeded, localLastReadMessage: $localLastReadMessage")
-        Log.d(
-            TAG,
-            "updateRemoteLastReadMessageIfNeeded, _uiState.value.conversation!!.lastReadMessage: " +
-                _uiState.value.conversation!!.lastReadMessage
-        )
+        val conversationLastReadMessage = _uiState.value.conversation?.lastReadMessage ?: return
+        Log.d(TAG, "updateRemoteLastReadMessageIfNeeded, conversation.lastReadMessage: $conversationLastReadMessage")
 
-        if (localLastReadMessage > _uiState.value.conversation!!.lastReadMessage) {
+        if (localLastReadMessage > conversationLastReadMessage) {
             Log.d(TAG, "updateRemoteLastReadMessageIfNeeded, setChatReadMessage...")
 
-            setChatReadMessage(credentials, url, localLastReadMessage)
+            setChatReadMessage(localLastReadMessage)
         }
     }
 
     /**
-     * Please use with caution to not spam the server
+     * Marks the chat as read up to [lastReadMessage]: the local conversation entry is updated
+     * immediately (optimistic, so the conversation list reflects it right away) while sending the
+     * marker to the server is delegated to [ReadMarkerSyncWorker], which retries transient
+     * failures with backoff. The server stays the authority — every room list sync re-asserts its
+     * read state, so a marker that ultimately could not be sent falls back to the server state
+     * instead of leaving the client diverged.
+     *
+     * [markPendingReadMarker] runs synchronously, before anything is launched, so the marker is
+     * armed the instant this returns: leaving the chat commonly races the conversation list's own
+     * resume-triggered sync, and that race is only guarded correctly if the pending marker already
+     * exists by the time the sync's response is merged. [updateLocalReadState] itself does two
+     * suspending database reads before writing - registering the marker only there, inside a
+     * launched coroutine, previously left a real gap (observed at ~250ms, more under main-thread
+     * contention) during which such a sync could see no pending marker yet and apply unguarded.
      */
-    fun setChatReadMessage(credentials: String, url: String, lastReadMessage: Int) {
-        chatNetworkDataSource.setChatReadMarker(credentials, url, lastReadMessage)
-            .subscribeOn(Schedulers.io())
-            .observeOn(AndroidSchedulers.mainThread())
-            .subscribe(object : Observer<GenericOverall> {
-                override fun onSubscribe(d: Disposable) {
-                    disposableSet.add(d)
-                }
-
-                override fun onError(e: Throwable) {
-                    Log.e(TAG, e.message, e)
-                }
-
-                override fun onComplete() {
-                    // unused atm
-                }
-
-                override fun onNext(t: GenericOverall) {
-                    // unused atm
-                }
-            })
+    fun setChatReadMessage(lastReadMessage: Int) {
+        if (!this::currentUser.isInitialized) {
+            return
+        }
+        chatRepository.markPendingReadMarker(lastReadMessage)
+        viewModelScope.launch {
+            chatRepository.updateLocalReadState(lastReadMessage)
+        }
+        ReadMarkerSyncWorker.enqueue(
+            context = NextcloudTalkApplication.sharedApplication!!.applicationContext,
+            userId = currentUser.id!!,
+            roomToken = chatRoomToken,
+            lastReadMessage = lastReadMessage
+        )
     }
 
     fun shareToNotes(credentials: String, url: String, message: String, displayName: String) {