From 91d34040671c735ff61dde52a4aeb0af4e0fd7fa Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 2 Aug 2026 18:36:03 +0200 Subject: [PATCH 01/27] refactor(chat): extract message sync into reusable component Move the fetch-and-persist core (getAndPersistMessages, persistChatMessagesAndHandleSystemMessages, updateBlocks) out of OfflineFirstChatRepository into a singleton ChatMessageSyncer that takes (user, roomToken, threadId) per call instead of relying on lateinit state set by ChatActivity. The repository delegates to it so open-chat and background paths share a single write path. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 492 ++++++++++++++++++ .../network/OfflineFirstChatRepository.kt | 424 ++------------- .../talk/chat/ui/VoiceRecordingLockFab.kt | 87 ++++ .../talk/dagger/modules/RepositoryModule.kt | 22 +- .../talk/utils/preview/ComposePreviewUtils.kt | 12 +- 5 files changed, 649 insertions(+), 388 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt create mode 100644 app/src/main/java/com/nextcloud/talk/chat/ui/VoiceRecordingLockFab.kt diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt new file mode 100644 index 00000000000..415d7c5bc14 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -0,0 +1,492 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Andy Scherzinger + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.chat.data.network + +import android.database.sqlite.SQLiteConstraintException +import android.util.Log +import com.nextcloud.talk.chat.data.model.ChatMessage +import com.nextcloud.talk.chat.domain.ChatPullResult +import com.nextcloud.talk.data.database.dao.ChatBlocksDao +import com.nextcloud.talk.data.database.dao.ChatMessagesDao +import com.nextcloud.talk.data.database.mappers.asEntity +import com.nextcloud.talk.data.database.model.ChatBlockEntity +import com.nextcloud.talk.data.database.model.ChatMessageEntity +import com.nextcloud.talk.data.network.NetworkMonitor +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.models.json.chat.ChatMessageJson +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOn +import retrofit2.HttpException +import javax.inject.Inject + +/** + * The chat message fetch-and-persist core, shared between the chat screen + * ([OfflineFirstChatRepository]) and background sync callers. + * + * The syncer holds no per-conversation state: every operation takes a [SyncTarget] describing the + * account, room and thread to sync, so it can be used for any room at any time — no open chat + * required. UI-bound side effects are reported through the optional [Events] listener. + */ +class ChatMessageSyncer @Inject constructor( + private val chatDao: ChatMessagesDao, + private val chatBlocksDao: ChatBlocksDao, + private val network: ChatNetworkDataSource, + private val networkMonitor: NetworkMonitor +) { + + /** + * Identifies the conversation (and optionally thread) a sync operation works on. + */ + data class SyncTarget( + val user: User, + val roomToken: String, + val threadId: Long?, + val credentials: String, + val urlForChatting: String + ) { + val internalConversationId: String = "${user.id}@$roomToken" + val accountId: Long + get() = user.id!! + } + + /** + * Side effects that only matter while a chat is on screen. Background callers can pass [NO_EVENTS]. + */ + interface Events { + suspend fun onLastCommonReadChanged(lastCommonRead: Int?) { + // no-op by default + } + + suspend fun onLoadingChanged(isLoading: Boolean) { + // no-op by default + } + + suspend fun onRoomRefreshNeeded() { + // no-op by default + } + + suspend fun onIncomingMessagesFromOthers() { + // no-op by default + } + } + + data class SyncOutcome(val persistedNewMessages: Boolean, val newestPersistedMessageId: Long?) + + fun pullMessagesFlow(target: SyncTarget, fieldMap: HashMap): Flow = + flow { + var attempts = 1 + + while (attempts < MAX_PULL_ATTEMPTS) { + runCatching { + network.pullChatMessages(target.credentials, target.urlForChatting, fieldMap) + }.fold( + onSuccess = { response -> + val result = when (response.code()) { + HTTP_CODE_OK -> ChatPullResult.Success( + messages = response.body()?.ocs?.data.orEmpty(), + lastCommonRead = response.headers()["X-Chat-Last-Common-Read"]?.toInt() + ) + HTTP_CODE_NOT_MODIFIED -> ChatPullResult.NotModified + HTTP_CODE_PRECONDITION_FAILED -> ChatPullResult.PreconditionFailed + else -> ChatPullResult.Error(HttpException(response)) + } + + emit(result) + return@flow + }, + onFailure = { e -> + Log.e(TAG, "Attempt $attempts failed", e) + attempts++ + fieldMap["limit"] = when (attempts) { + 2 -> RETRY_LIMIT_SECOND_ATTEMPT + 3 -> RETRY_LIMIT_THIRD_ATTEMPT + else -> RETRY_LIMIT_FALLBACK_ATTEMPT + } + } + ) + } + + emit(ChatPullResult.Error(IllegalStateException("All attempts failed"))) + }.flowOn(Dispatchers.IO) + + /** + * Pulls messages from the server as described by [fieldMap], persists them and updates the + * chat blocks of [target]. + */ + suspend fun pullAndPersistMessages( + target: SyncTarget, + fieldMap: HashMap, + events: Events = NO_EVENTS + ): SyncOutcome { + val isLongPoll = (fieldMap["timeout"] ?: 0) > 0 + if (!isLongPoll) events.onLoadingChanged(true) + try { + if (!networkMonitor.isOnline.value) { + Log.d(TAG, "Device is offline, can't load chat messages from server") + } + + val queriedMessageId = fieldMap["lastKnownMessageId"] + val lookIntoFuture = fieldMap["lookIntoFuture"] == 1 + + val result = pullMessagesFlow(target, fieldMap).first() + + when (result) { + is ChatPullResult.Success -> { + events.onLastCommonReadChanged(result.lastCommonRead) + + val hasHistory = getHasHistory(HTTP_CODE_OK, lookIntoFuture) + + Log.d( + TAG, + "internalConv=${target.internalConversationId} statusCode=$HTTP_CODE_OK " + + "lookIntoFuture=$lookIntoFuture hasHistory=$hasHistory " + + "queriedMessageId=$queriedMessageId" + ) + + val blockContainingQueriedMessage: ChatBlockEntity? = getBlockOfMessage(target, queriedMessageId) + + blockContainingQueriedMessage?.takeIf { !hasHistory }?.apply { + this.hasHistory = false + chatBlocksDao.upsertChatBlock(this) + Log.d(TAG, "End of chat reached, set hasHistory=false") + } + + if (result.messages.isNotEmpty()) { + val newestPersistedId = updateMessagesData( + target, + result.messages, + blockContainingQueriedMessage, + lookIntoFuture, + hasHistory, + events + ) + return SyncOutcome(persistedNewMessages = true, newestPersistedMessageId = newestPersistedId) + } else { + Log.d(TAG, "No new messages to update") + return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + } + } + + is ChatPullResult.NotModified -> { + Log.d(TAG, "Server returned NOT_MODIFIED, nothing to update") + return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + } + + is ChatPullResult.PreconditionFailed -> { + Log.d(TAG, "Server returned PRECONDITION_FAILED, nothing to update") + return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + } + + is ChatPullResult.Error -> { + Log.e(TAG, "Error pulling messages from server", result.throwable) + return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + } + } + } finally { + if (!isLongPoll) events.onLoadingChanged(false) + } + } + + private suspend fun updateMessagesData( + target: SyncTarget, + chatMessagesJson: List, + blockContainingQueriedMessage: ChatBlockEntity?, + lookIntoFuture: Boolean, + hasHistory: Boolean, + events: Events + ): Long? { + val chatMessageEntities = + persistChatMessagesAndHandleSystemMessages(target, chatMessagesJson, emitOnIncoming = lookIntoFuture, events) + + val oldestIdFromSync = chatMessageEntities.minByOrNull { it.id }!!.id + val newestIdFromSync = chatMessageEntities.maxByOrNull { it.id }!!.id + Log.d(TAG, "oldestIdFromSync: $oldestIdFromSync") + Log.d(TAG, "newestIdFromSync: $newestIdFromSync") + + var oldestMessageIdForNewChatBlock = oldestIdFromSync + var newestMessageIdForNewChatBlock = newestIdFromSync + + if (blockContainingQueriedMessage != null) { + if (lookIntoFuture) { + val oldestMessageIdFromBlockOfQueriedMessage = blockContainingQueriedMessage.oldestMessageId + Log.d(TAG, "oldestMessageIdFromBlockOfQueriedMessage: $oldestMessageIdFromBlockOfQueriedMessage") + oldestMessageIdForNewChatBlock = oldestMessageIdFromBlockOfQueriedMessage + } else { + val newestMessageIdFromBlockOfQueriedMessage = blockContainingQueriedMessage.newestMessageId + Log.d(TAG, "newestMessageIdFromBlockOfQueriedMessage: $newestMessageIdFromBlockOfQueriedMessage") + newestMessageIdForNewChatBlock = newestMessageIdFromBlockOfQueriedMessage + } + } + + Log.d(TAG, "oldestMessageIdForNewChatBlock: $oldestMessageIdForNewChatBlock") + Log.d(TAG, "newestMessageIdForNewChatBlock: $newestMessageIdForNewChatBlock") + + val newChatBlock = ChatBlockEntity( + internalConversationId = target.internalConversationId, + accountId = target.accountId, + token = target.roomToken, + threadId = target.threadId, + oldestMessageId = oldestMessageIdForNewChatBlock, + newestMessageId = newestMessageIdForNewChatBlock, + hasHistory = hasHistory + ) + updateBlocks(target, newChatBlock) + + return newestIdFromSync + } + + suspend fun persistChatMessagesAndHandleSystemMessages( + target: SyncTarget, + chatMessages: List, + emitOnIncoming: Boolean = false, + events: Events = NO_EVENTS + ): List { + handleSystemMessagesThatAffectDatabase(target, chatMessages, events) + + val chatMessageEntities = chatMessages.map { + it.asEntity(target.accountId) + } + + try { + chatDao.upsertChatMessagesAndDeleteTemp(target.internalConversationId, chatMessageEntities) + } catch (e: SQLiteConstraintException) { + // Skipped persisting messages: conversation $internalConversationId not in DB yet. + // This avoids "SQLiteConstraintException: FOREIGN KEY constraint failed". + // It may happen when a notification for a newly created conversation is opened. The websocket was just + // faster than the API request so no conversation for the message exists yet. Just swallow the exception + // and let the insurance request handle it. + Log.w(TAG, "Skip persisting messages from signaling: conversation not in DB yet. Swallowed exception: $e") + return emptyList() + } + + if (emitOnIncoming) { + val hasIncomingFromOther = chatMessages.any { msg -> + msg.systemMessageType == ChatMessage.SystemMessageType.DUMMY && + msg.actorId != target.user.userId + } + if (hasIncomingFromOther) { + events.onIncomingMessagesFromOthers() + } + } + + return chatMessageEntities + } + + /** + * Returns true if all system messages do not require translation. + * Ignores other message types. + */ + fun isUntranslatedSystemMessage(messagesJson: List): Boolean = + messagesJson.all { + it.systemMessageType == ChatMessage.SystemMessageType.DUMMY || + it.systemMessageType in ChatMessage.SYSTEM_MESSAGE_TYPE_UNTRANSLATED + } + + private suspend fun handleSystemMessagesThatAffectDatabase( + target: SyncTarget, + messagesJson: List, + events: Events + ) { + var needsRoomRefresh = false + messagesJson.forEach { messageJson -> + when (messageJson.systemMessageType) { + ChatMessage.SystemMessageType.REACTION, + ChatMessage.SystemMessageType.REACTION_REVOKED, + ChatMessage.SystemMessageType.REACTION_DELETED -> + // Signaling does not include reactionsSelf; derive it so the self-reaction + // border stays correct regardless of whether signaling or the API response lands first. + upsertParentMessage(target, messageJson, deriveReactions = true) + + ChatMessage.SystemMessageType.MESSAGE_DELETED, + ChatMessage.SystemMessageType.POLL_VOTED, + ChatMessage.SystemMessageType.MESSAGE_EDITED -> + upsertParentMessage(target, messageJson) + + ChatMessage.SystemMessageType.LOBBY_NONE, + ChatMessage.SystemMessageType.LOBBY_NON_MODERATORS, + ChatMessage.SystemMessageType.LOBBY_OPEN_TO_EVERYONE -> needsRoomRefresh = true + + ChatMessage.SystemMessageType.CLEARED_CHAT -> { + // for lookIntoFuture just deleting everything would be fine. + // But lets say we did not open the chat for a while and in between it was cleared. + // We just load the last messages but this don't contain the system message. + // We scroll up and load the system message. Deleting everything is not an option as we + // would loose the messages that we want to keep. We only want to + // delete the messages and chatBlocks older than the system message. + chatDao.deleteMessagesOlderThan(target.internalConversationId, messageJson.id) + chatBlocksDao.deleteChatBlocksOlderThan(target.internalConversationId, messageJson.id) + } + + ChatMessage.SystemMessageType.MESSAGE_PINNED, + ChatMessage.SystemMessageType.MESSAGE_UNPINNED -> needsRoomRefresh = true + + else -> {} + } + } + if (needsRoomRefresh) events.onRoomRefreshNeeded() + } + + // the parent message is always the newest state, no matter how old the system message is. + // that's why we can just take the parent, update it in DB and update the UI + private suspend fun upsertParentMessage( + target: SyncTarget, + messageJson: ChatMessageJson, + deriveReactions: Boolean = false + ) { + val parentMessageJson = messageJson.parentMessage ?: return + parentMessageJson.message ?: return + val parentMessageEntity = parentMessageJson.asEntity(target.accountId) + + // Preserve parentMessageId if missing in server response but present in local DB + val existingEntity = chatDao.getChatMessageEntity(target.internalConversationId, parentMessageJson.id) + if (existingEntity != null && parentMessageEntity.parentMessageId == null) { + parentMessageEntity.parentMessageId = existingEntity.parentMessageId + } + + if (deriveReactions) { + parentMessageEntity.reactionsSelf = + deriveReactionsSelf(target, messageJson, existingEntity) + } + + chatDao.upsertChatMessage(parentMessageEntity) + } + + /** + * Derives the correct reactionsSelf list for a parent message when a reaction system message arrives via + * signaling. The signaling payload does not include reactionsSelf, so we preserve the existing DB state and + * apply a targeted update only if the actor of the system message is the current user. + * + * The emoji is always in the message field (messageParameters is empty for all reaction system messages). + * + * This handles the race condition where signaling may arrive before or after + * ReactionsRepositoryImpl has written the optimistic local update. + */ + private fun deriveReactionsSelf( + target: SyncTarget, + systemMessageJson: ChatMessageJson, + existingEntity: ChatMessageEntity? + ): ArrayList { + val reactionsSelf = ArrayList(existingEntity?.reactionsSelf ?: emptyList()) + val isCurrentUserActor = systemMessageJson.actorId == target.user.userId && + systemMessageJson.actorType == "users" + + if (isCurrentUserActor) { + val emoji = systemMessageJson.message + if (emoji != null) { + when (systemMessageJson.systemMessageType) { + ChatMessage.SystemMessageType.REACTION -> { + if (!reactionsSelf.contains(emoji)) reactionsSelf.add(emoji) + } + + ChatMessage.SystemMessageType.REACTION_REVOKED, + ChatMessage.SystemMessageType.REACTION_DELETED -> reactionsSelf.remove(emoji) + else -> {} + } + } + } + + return reactionsSelf + } + + /** + * 304 is returned when oldest message of chat was queried or when long polling request returned with no + * modification. hasHistory is only set to false, when 304 was returned for the the oldest message + */ + private fun getHasHistory(statusCode: Int, lookIntoFuture: Boolean): Boolean = + if (statusCode == HTTP_CODE_NOT_MODIFIED) { + lookIntoFuture + } else { + true + } + + suspend fun getBlockOfMessage(target: SyncTarget, queriedMessageId: Int?): ChatBlockEntity? { + var blockContainingQueriedMessage: ChatBlockEntity? = null + if (queriedMessageId != null) { + val blocksContainingQueriedMessage = + chatBlocksDao.getChatBlocksContainingMessageId( + internalConversationId = target.internalConversationId, + threadId = target.threadId, + messageId = queriedMessageId.toLong() + ) + + val chatBlocks = blocksContainingQueriedMessage.first() + if (chatBlocks.size > 1) { + Log.w(TAG, "multiple chat blocks with messageId $queriedMessageId were found") + } + + blockContainingQueriedMessage = if (chatBlocks.isNotEmpty()) { + chatBlocks.first() + } else { + null + } + } + return blockContainingQueriedMessage + } + + suspend fun updateBlocks(target: SyncTarget, chatBlock: ChatBlockEntity) { + chatBlocksDao.upsertChatBlock(chatBlock) + + val connectedChatBlocks = + chatBlocksDao.getConnectedChatBlocks( + internalConversationId = target.internalConversationId, + threadId = target.threadId, + oldestMessageId = chatBlock.oldestMessageId, + newestMessageId = chatBlock.newestMessageId + ).first() + + if (connectedChatBlocks.size == 1) { + Log.d(TAG, "This chatBlock is not connected to others") + val chatBlockFromDb = connectedChatBlocks[0] + Log.d(TAG, "chatBlockFromDb.oldestMessageId: " + chatBlockFromDb.oldestMessageId) + Log.d(TAG, "chatBlockFromDb.newestMessageId: " + chatBlockFromDb.newestMessageId) + } else if (connectedChatBlocks.size > 1) { + Log.d(TAG, "Found " + connectedChatBlocks.size + " chat blocks that are connected") + val oldestIdFromDbChatBlocks = + connectedChatBlocks.minByOrNull { it.oldestMessageId }!!.oldestMessageId + val newestIdFromDbChatBlocks = + connectedChatBlocks.maxByOrNull { it.newestMessageId }!!.newestMessageId + + val hasNoHistory = connectedChatBlocks.any { !it.hasHistory } + val hasHistory = !hasNoHistory + Log.d(TAG, "hasHistory = $hasHistory") + + val newChatBlock = ChatBlockEntity( + internalConversationId = target.internalConversationId, + accountId = target.accountId, + token = target.roomToken, + threadId = target.threadId, + oldestMessageId = oldestIdFromDbChatBlocks, + newestMessageId = newestIdFromDbChatBlocks, + hasHistory = hasHistory + ) + chatBlocksDao.replaceConnectedChatBlocks(connectedChatBlocks, newChatBlock) + Log.d(TAG, "A new chat block was created that covers all the range of the found chatblocks") + Log.d(TAG, "new chatBlock - oldest MessageId: $oldestIdFromDbChatBlocks") + Log.d(TAG, "new chatBlock - newest MessageId: $newestIdFromDbChatBlocks") + } else { + Log.d(TAG, "No chat block found ....") + } + } + + companion object { + val TAG: String = ChatMessageSyncer::class.java.simpleName + + val NO_EVENTS: Events = object : Events {} + + private const val HTTP_CODE_OK: Int = 200 + private const val HTTP_CODE_NOT_MODIFIED = 304 + private const val HTTP_CODE_PRECONDITION_FAILED = 412 + private const val MAX_PULL_ATTEMPTS = 5 + private const val RETRY_LIMIT_SECOND_ATTEMPT = 50 + private const val RETRY_LIMIT_THIRD_ATTEMPT = 10 + private const val RETRY_LIMIT_FALLBACK_ATTEMPT = 5 + } +} diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index ef2f10ead0c..22ae6554672 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -8,12 +8,10 @@ package com.nextcloud.talk.chat.data.network -import android.database.sqlite.SQLiteConstraintException 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.chat.domain.ChatPullResult import com.nextcloud.talk.data.database.dao.ChatBlocksDao import com.nextcloud.talk.data.database.dao.ChatMessagesDao import com.nextcloud.talk.data.database.mappers.asEntity @@ -33,7 +31,6 @@ import com.nextcloud.talk.models.json.generic.GenericOverall import com.nextcloud.talk.models.json.participants.Participant import com.nextcloud.talk.utils.bundle.BundleKeys import com.nextcloud.talk.utils.message.SendMessageUtils -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow @@ -47,10 +44,8 @@ import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.take -import retrofit2.HttpException import java.io.IOException import javax.inject.Inject @@ -60,7 +55,8 @@ class OfflineFirstChatRepository @Inject constructor( private val chatDao: ChatMessagesDao, private val chatBlocksDao: ChatBlocksDao, private val network: ChatNetworkDataSource, - private val networkMonitor: NetworkMonitor + private val networkMonitor: NetworkMonitor, + private val syncer: ChatMessageSyncer ) : ChatMessageRepository { lateinit var currentUser: User @@ -151,6 +147,34 @@ class OfflineFirstChatRepository @Inject constructor( this.conversationModel = conversationModel } + private val syncTarget: ChatMessageSyncer.SyncTarget + get() = ChatMessageSyncer.SyncTarget( + user = currentUser, + roomToken = roomToken, + threadId = threadId, + credentials = credentials, + urlForChatting = urlForChatting + ) + + private val syncEvents = object : ChatMessageSyncer.Events { + override suspend fun onLastCommonReadChanged(lastCommonRead: Int?) { + newXChatLastCommonRead = lastCommonRead + updateUiForLastCommonRead() + } + + override suspend fun onLoadingChanged(isLoading: Boolean) { + _isLoadingFlow.value = isLoading + } + + override suspend fun onRoomRefreshNeeded() { + _roomRefreshFlow.emit(Unit) + } + + override suspend fun onIncomingMessagesFromOthers() { + _incomingMessageFlow.emit(Unit) + } + } + override suspend fun loadInitialMessages(withNetworkParams: Bundle, isChatRelaySupported: Boolean) { logger.d(TAG, "---- loadInitialMessages ------------") cleanupExpiredMessages() @@ -334,7 +358,7 @@ class OfflineFirstChatRepository @Inject constructor( Log.d(TAG, "Starting online request for loadMoreMessages") getAndPersistMessages(withNetworkParams) - return getBlockOfMessage(anchorMessageId.toInt())?.let { + return syncer.getBlockOfMessage(syncTarget, anchorMessageId.toInt())?.let { ChatMessageRepository.MessagesRange( oldestMessageId = it.oldestMessageId, newestMessageId = it.newestMessageId @@ -431,7 +455,7 @@ class OfflineFirstChatRepository @Inject constructor( newestMessageId = newestId, hasHistory = true ) - updateBlocks(block) + syncer.updateBlocks(syncTarget, block) ChatMessageRepository.MessagesRange( oldestMessageId = oldestId, @@ -479,351 +503,17 @@ class OfflineFirstChatRepository @Inject constructor( } } - fun pullMessagesFlow(bundle: Bundle): Flow = - flow { - val fieldMap = bundle.getSerializable(BundleKeys.KEY_FIELD_MAP) as HashMap - var attempts = 1 - - while (attempts < MAX_PULL_ATTEMPTS) { - runCatching { - network.pullChatMessages(credentials, urlForChatting, fieldMap) - }.fold( - onSuccess = { response -> - val result = when (response.code()) { - HTTP_CODE_OK -> ChatPullResult.Success( - messages = response.body()?.ocs?.data.orEmpty(), - lastCommonRead = response.headers()["X-Chat-Last-Common-Read"]?.toInt() - ) - HTTP_CODE_NOT_MODIFIED -> ChatPullResult.NotModified - HTTP_CODE_PRECONDITION_FAILED -> ChatPullResult.PreconditionFailed - else -> ChatPullResult.Error(HttpException(response)) - } - - emit(result) - return@flow - }, - onFailure = { e -> - Log.e(TAG, "Attempt $attempts failed", e) - attempts++ - fieldMap["limit"] = when (attempts) { - 2 -> RETRY_LIMIT_SECOND_ATTEMPT - 3 -> RETRY_LIMIT_THIRD_ATTEMPT - else -> RETRY_LIMIT_FALLBACK_ATTEMPT - } - } - ) - } - - emit(ChatPullResult.Error(IllegalStateException("All attempts failed"))) - }.flowOn(Dispatchers.IO) - private suspend fun getAndPersistMessages(bundle: Bundle): Boolean { val fieldMap = bundle.getSerializable(BundleKeys.KEY_FIELD_MAP) as HashMap - val isLongPoll = (fieldMap["timeout"] ?: 0) > 0 - if (!isLongPoll) _isLoadingFlow.value = true - try { - if (!networkMonitor.isOnline.value) { - Log.d(TAG, "Device is offline, can't load chat messages from server") - } - - val queriedMessageId = fieldMap["lastKnownMessageId"] - val lookIntoFuture = fieldMap["lookIntoFuture"] == 1 - - val result = pullMessagesFlow(bundle).first() - - when (result) { - is ChatPullResult.Success -> { - newXChatLastCommonRead = result.lastCommonRead - updateUiForLastCommonRead() - - val hasHistory = getHasHistory(HTTP_CODE_OK, lookIntoFuture) - - Log.d( - TAG, - "internalConv=$internalConversationId statusCode=${HTTP_CODE_OK} " + - "lookIntoFuture=$lookIntoFuture hasHistory=$hasHistory " + - "queriedMessageId=$queriedMessageId" - ) - - val blockContainingQueriedMessage: ChatBlockEntity? = getBlockOfMessage(queriedMessageId) - - blockContainingQueriedMessage?.takeIf { !hasHistory }?.apply { - this.hasHistory = false - chatBlocksDao.upsertChatBlock(this) - Log.d(TAG, "End of chat reached, set hasHistory=false") - } - - if (result.messages.isNotEmpty()) { - updateMessagesData( - result.messages, - blockContainingQueriedMessage, - lookIntoFuture, - hasHistory - ) - return true - } else { - Log.d(TAG, "No new messages to update") - return false - } - } - - is ChatPullResult.NotModified -> { - Log.d(TAG, "Server returned NOT_MODIFIED, nothing to update") - return false - } - - is ChatPullResult.PreconditionFailed -> { - Log.d(TAG, "Server returned PRECONDITION_FAILED, nothing to update") - return false - } - - is ChatPullResult.Error -> { - Log.e(TAG, "Error pulling messages from server", result.throwable) - return false - } - } - } finally { - if (!isLongPoll) _isLoadingFlow.value = false - } - } - - private suspend fun OfflineFirstChatRepository.updateMessagesData( - chatMessagesJson: List, - blockContainingQueriedMessage: ChatBlockEntity?, - lookIntoFuture: Boolean, - hasHistory: Boolean - ) { - val chatMessageEntities = - persistChatMessagesAndHandleSystemMessages(chatMessagesJson, emitOnIncoming = lookIntoFuture) - - if (chatMessageEntities.isEmpty()) { - Log.w(TAG, "No messages were persisted, skipping chat block update") - return - } - - val oldestIdFromSync = chatMessageEntities.minByOrNull { it.id }!!.id - val newestIdFromSync = chatMessageEntities.maxByOrNull { it.id }!!.id - Log.d(TAG, "oldestIdFromSync: $oldestIdFromSync") - Log.d(TAG, "newestIdFromSync: $newestIdFromSync") - - latestKnownMessageIdFromSync = maxOf(latestKnownMessageIdFromSync, newestIdFromSync) - - var oldestMessageIdForNewChatBlock = oldestIdFromSync - var newestMessageIdForNewChatBlock = newestIdFromSync - - if (blockContainingQueriedMessage != null) { - if (lookIntoFuture) { - val oldestMessageIdFromBlockOfQueriedMessage = blockContainingQueriedMessage.oldestMessageId - Log.d(TAG, "oldestMessageIdFromBlockOfQueriedMessage: $oldestMessageIdFromBlockOfQueriedMessage") - oldestMessageIdForNewChatBlock = oldestMessageIdFromBlockOfQueriedMessage - } else { - val newestMessageIdFromBlockOfQueriedMessage = blockContainingQueriedMessage.newestMessageId - Log.d(TAG, "newestMessageIdFromBlockOfQueriedMessage: $newestMessageIdFromBlockOfQueriedMessage") - newestMessageIdForNewChatBlock = newestMessageIdFromBlockOfQueriedMessage - } + val outcome = syncer.pullAndPersistMessages(syncTarget, fieldMap, syncEvents) + outcome.newestPersistedMessageId?.let { + latestKnownMessageIdFromSync = maxOf(latestKnownMessageIdFromSync, it) } - - Log.d(TAG, "oldestMessageIdForNewChatBlock: $oldestMessageIdForNewChatBlock") - Log.d(TAG, "newestMessageIdForNewChatBlock: $newestMessageIdForNewChatBlock") - - val newChatBlock = ChatBlockEntity( - internalConversationId = internalConversationId, - accountId = conversationModel.accountId, - token = conversationModel.token, - threadId = threadId, - oldestMessageId = oldestMessageIdForNewChatBlock, - newestMessageId = newestMessageIdForNewChatBlock, - hasHistory = hasHistory - ) - updateBlocks(newChatBlock) + return outcome.persistedNewMessages } - /** - * Returns true if all system messages do not require translation. - * Ignores other message types. - */ private fun isUntranslatedSystemMessage(messagesJson: List): Boolean = - messagesJson.all { - it.systemMessageType == ChatMessage.SystemMessageType.DUMMY || - it.systemMessageType in ChatMessage.SYSTEM_MESSAGE_TYPE_UNTRANSLATED - } - - private suspend fun handleSystemMessagesThatAffectDatabase(messagesJson: List) { - var needsRoomRefresh = false - messagesJson.forEach { messageJson -> - when (messageJson.systemMessageType) { - ChatMessage.SystemMessageType.REACTION, - ChatMessage.SystemMessageType.REACTION_REVOKED, - ChatMessage.SystemMessageType.REACTION_DELETED -> - // Signaling does not include reactionsSelf; derive it so the self-reaction - // border stays correct regardless of whether signaling or the API response lands first. - upsertParentMessage(messageJson, deriveReactions = true) - - ChatMessage.SystemMessageType.MESSAGE_DELETED, - ChatMessage.SystemMessageType.POLL_VOTED, - ChatMessage.SystemMessageType.MESSAGE_EDITED -> - upsertParentMessage(messageJson) - - ChatMessage.SystemMessageType.LOBBY_NONE, - ChatMessage.SystemMessageType.LOBBY_NON_MODERATORS, - ChatMessage.SystemMessageType.LOBBY_OPEN_TO_EVERYONE -> needsRoomRefresh = true - - ChatMessage.SystemMessageType.CLEARED_CHAT -> { - // for lookIntoFuture just deleting everything would be fine. - // But lets say we did not open the chat for a while and in between it was cleared. - // We just load the last messages but this don't contain the system message. - // We scroll up and load the system message. Deleting everything is not an option as we - // would loose the messages that we want to keep. We only want to - // delete the messages and chatBlocks older than the system message. - chatDao.deleteMessagesOlderThan(internalConversationId, messageJson.id) - chatBlocksDao.deleteChatBlocksOlderThan(internalConversationId, messageJson.id) - } - - ChatMessage.SystemMessageType.MESSAGE_PINNED, - ChatMessage.SystemMessageType.MESSAGE_UNPINNED -> needsRoomRefresh = true - - else -> {} - } - } - if (needsRoomRefresh) _roomRefreshFlow.emit(Unit) - } - - // the parent message is always the newest state, no matter how old the system message is. - // that's why we can just take the parent, update it in DB and update the UI - private suspend fun upsertParentMessage(messageJson: ChatMessageJson, deriveReactions: Boolean = false) { - val parentMessageJson = messageJson.parentMessage ?: return - parentMessageJson.message ?: return - val parentMessageEntity = parentMessageJson.asEntity(currentUser.id!!) - - // Preserve parentMessageId if missing in server response but present in local DB - val existingEntity = chatDao.getChatMessageEntity(internalConversationId, parentMessageJson.id) - if (existingEntity != null && parentMessageEntity.parentMessageId == null) { - parentMessageEntity.parentMessageId = existingEntity.parentMessageId - } - - if (deriveReactions) { - parentMessageEntity.reactionsSelf = - deriveReactionsSelf(messageJson, existingEntity) - } - - chatDao.upsertChatMessage(parentMessageEntity) - } - - /** - * Derives the correct reactionsSelf list for a parent message when a reaction system message arrives via - * signaling. The signaling payload does not include reactionsSelf, so we preserve the existing DB state and - * apply a targeted update only if the actor of the system message is the current user. - * - * The emoji is always in the message field (messageParameters is empty for all reaction system messages). - * - * This handles the race condition where signaling may arrive before or after - * ReactionsRepositoryImpl has written the optimistic local update. - */ - private fun deriveReactionsSelf( - systemMessageJson: ChatMessageJson, - existingEntity: ChatMessageEntity? - ): ArrayList { - val reactionsSelf = ArrayList(existingEntity?.reactionsSelf ?: emptyList()) - val isCurrentUserActor = systemMessageJson.actorId == currentUser.userId && - systemMessageJson.actorType == "users" - - if (isCurrentUserActor) { - val emoji = systemMessageJson.message - if (emoji != null) { - when (systemMessageJson.systemMessageType) { - ChatMessage.SystemMessageType.REACTION -> { - if (!reactionsSelf.contains(emoji)) reactionsSelf.add(emoji) - } - - ChatMessage.SystemMessageType.REACTION_REVOKED, - ChatMessage.SystemMessageType.REACTION_DELETED -> reactionsSelf.remove(emoji) - else -> {} - } - } - } - - return reactionsSelf - } - - /** - * 304 is returned when oldest message of chat was queried or when long polling request returned with no - * modification. hasHistory is only set to false, when 304 was returned for the the oldest message - */ - private fun getHasHistory(statusCode: Int, lookIntoFuture: Boolean): Boolean = - if (statusCode == HTTP_CODE_NOT_MODIFIED) { - lookIntoFuture - } else { - true - } - - private suspend fun getBlockOfMessage(queriedMessageId: Int?): ChatBlockEntity? { - var blockContainingQueriedMessage: ChatBlockEntity? = null - if (queriedMessageId != null) { - val blocksContainingQueriedMessage = - chatBlocksDao.getChatBlocksContainingMessageId( - internalConversationId = internalConversationId, - threadId = threadId, - messageId = queriedMessageId.toLong() - ) - - val chatBlocks = blocksContainingQueriedMessage.first() - if (chatBlocks.size > 1) { - Log.w(TAG, "multiple chat blocks with messageId $queriedMessageId were found") - } - - blockContainingQueriedMessage = if (chatBlocks.isNotEmpty()) { - chatBlocks.first() - } else { - null - } - } - return blockContainingQueriedMessage - } - - private suspend fun updateBlocks(chatBlock: ChatBlockEntity) { - chatBlocksDao.upsertChatBlock(chatBlock) - - val connectedChatBlocks = - chatBlocksDao.getConnectedChatBlocks( - internalConversationId = internalConversationId, - threadId = threadId, - oldestMessageId = chatBlock.oldestMessageId, - newestMessageId = chatBlock.newestMessageId - ).first() - - if (connectedChatBlocks.size == 1) { - Log.d(TAG, "This chatBlock is not connected to others") - val chatBlockFromDb = connectedChatBlocks[0] - Log.d(TAG, "chatBlockFromDb.oldestMessageId: " + chatBlockFromDb.oldestMessageId) - Log.d(TAG, "chatBlockFromDb.newestMessageId: " + chatBlockFromDb.newestMessageId) - } else if (connectedChatBlocks.size > 1) { - Log.d(TAG, "Found " + connectedChatBlocks.size + " chat blocks that are connected") - val oldestIdFromDbChatBlocks = - connectedChatBlocks.minByOrNull { it.oldestMessageId }!!.oldestMessageId - val newestIdFromDbChatBlocks = - connectedChatBlocks.maxByOrNull { it.newestMessageId }!!.newestMessageId - - val hasNoHistory = connectedChatBlocks.any { !it.hasHistory } - val hasHistory = !hasNoHistory - Log.d(TAG, "hasHistory = $hasHistory") - - val newChatBlock = ChatBlockEntity( - internalConversationId = internalConversationId, - accountId = conversationModel.accountId, - token = conversationModel.token, - threadId = threadId, - oldestMessageId = oldestIdFromDbChatBlocks, - newestMessageId = newestIdFromDbChatBlocks, - hasHistory = hasHistory - ) - chatBlocksDao.replaceConnectedChatBlocks(connectedChatBlocks, newChatBlock) - Log.d(TAG, "A new chat block was created that covers all the range of the found chatblocks") - Log.d(TAG, "new chatBlock - oldest MessageId: $oldestIdFromDbChatBlocks") - Log.d(TAG, "new chatBlock - newest MessageId: $newestIdFromDbChatBlocks") - } else { - Log.d(TAG, "No chat block found ....") - } - } + syncer.isUntranslatedSystemMessage(messagesJson) override fun handleOnPause() { itIsPaused = true @@ -1092,37 +782,8 @@ class OfflineFirstChatRepository @Inject constructor( suspend fun persistChatMessagesAndHandleSystemMessages( chatMessages: List, emitOnIncoming: Boolean = false - ): List { - handleSystemMessagesThatAffectDatabase(chatMessages) - - val chatMessageEntities = chatMessages.map { - it.asEntity(currentUser.id!!) - } - - try { - chatDao.upsertChatMessagesAndDeleteTemp(internalConversationId, chatMessageEntities) - } catch (e: SQLiteConstraintException) { - // Skipped persisting messages: conversation $internalConversationId not in DB yet. - // This avoids "SQLiteConstraintException: FOREIGN KEY constraint failed". - // It may happen when a notification for a newly created conversation is opened. The websocket was just - // faster than the API request so no conversation for the message exists yet. Just swallow the exception - // and let the insurance request handle it. - Log.w(TAG, "Skip persisting messages from signaling: conversation not in DB yet. Swallowed exception: $e") - return emptyList() - } - - if (emitOnIncoming) { - val hasIncomingFromOther = chatMessages.any { msg -> - msg.systemMessageType == ChatMessage.SystemMessageType.DUMMY && - msg.actorId != currentUser.userId - } - if (hasIncomingFromOther) { - _incomingMessageFlow.emit(Unit) - } - } - - return chatMessageEntities - } + ): List = + syncer.persistChatMessagesAndHandleSystemMessages(syncTarget, chatMessages, emitOnIncoming, syncEvents) override fun observeLatestMessages(internalConversationId: String): Flow> = chatBlocksDao @@ -1298,15 +959,8 @@ class OfflineFirstChatRepository @Inject constructor( companion object { val TAG: String = OfflineFirstChatRepository::class.java.simpleName - private const val HTTP_CODE_OK: Int = 200 - private const val HTTP_CODE_NOT_MODIFIED = 304 - private const val HTTP_CODE_PRECONDITION_FAILED = 412 private const val HALF_SECOND = 500L private const val DEFAULT_MESSAGES_LIMIT = 100 - private const val MAX_PULL_ATTEMPTS = 5 - private const val RETRY_LIMIT_SECOND_ATTEMPT = 50 - private const val RETRY_LIMIT_THIRD_ATTEMPT = 10 - private const val RETRY_LIMIT_FALLBACK_ATTEMPT = 5 private const val MILLIES = 1000L private const val INSURANCE_REQUEST_DELAY = 2 * 60 * MILLIES diff --git a/app/src/main/java/com/nextcloud/talk/chat/ui/VoiceRecordingLockFab.kt b/app/src/main/java/com/nextcloud/talk/chat/ui/VoiceRecordingLockFab.kt new file mode 100644 index 00000000000..ba73dcff8a2 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/chat/ui/VoiceRecordingLockFab.kt @@ -0,0 +1,87 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.chat.ui + +import android.content.res.Configuration +import androidx.compose.animation.AnimatedVisibility +import androidx.compose.animation.fadeIn +import androidx.compose.animation.fadeOut +import androidx.compose.animation.scaleIn +import androidx.compose.animation.scaleOut +import androidx.compose.foundation.isSystemInDarkTheme +import androidx.compose.material3.FloatingActionButton +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Surface +import androidx.compose.material3.darkColorScheme +import androidx.compose.material3.lightColorScheme +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.graphicsLayer +import androidx.compose.ui.res.painterResource +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.tooling.preview.Preview +import com.nextcloud.talk.R + +@Composable +fun VoiceRecordingLockFab(visible: Boolean, offsetY: Float, modifier: Modifier = Modifier) { + AnimatedVisibility( + visible = visible, + modifier = modifier, + enter = scaleIn() + fadeIn(), + exit = scaleOut() + fadeOut() + ) { + FloatingActionButton( + onClick = {}, + modifier = Modifier.graphicsLayer { translationY = offsetY }, + containerColor = MaterialTheme.colorScheme.primaryContainer, + contentColor = MaterialTheme.colorScheme.onPrimaryContainer + ) { + Icon( + painter = painterResource(R.drawable.ic_lock_open_grey600_24dp), + contentDescription = stringResource(R.string.continuous_voice_message_recording) + ) + } + } +} + +private const val PREVIEW_DRAG_OFFSET_PX = -80f + +@Preview(name = "Visible · default position · Light") +@Preview(name = "Visible · default position · Dark", uiMode = Configuration.UI_MODE_NIGHT_YES) +@Composable +private fun VisibleDefaultPreview() { + val colorScheme = if (isSystemInDarkTheme()) darkColorScheme() else lightColorScheme() + MaterialTheme(colorScheme = colorScheme) { + Surface { + VoiceRecordingLockFab(visible = true, offsetY = 0f) + } + } +} + +@Preview(name = "Visible · mid-drag · Light") +@Preview(name = "Visible · mid-drag · Dark", uiMode = Configuration.UI_MODE_NIGHT_YES) +@Composable +private fun VisibleDraggedPreview() { + val colorScheme = if (isSystemInDarkTheme()) darkColorScheme() else lightColorScheme() + MaterialTheme(colorScheme = colorScheme) { + Surface { + VoiceRecordingLockFab(visible = true, offsetY = PREVIEW_DRAG_OFFSET_PX) + } + } +} + +@Preview(name = "Hidden · Light") +@Composable +private fun HiddenPreview() { + MaterialTheme(colorScheme = lightColorScheme()) { + Surface { + VoiceRecordingLockFab(visible = false, offsetY = 0f) + } + } +} diff --git a/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt b/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt index e1c91d7f403..773277937ff 100644 --- a/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt +++ b/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt @@ -16,6 +16,7 @@ import com.nextcloud.talk.account.data.network.NetworkLoginDataSource import com.nextcloud.talk.api.NcApi import com.nextcloud.talk.api.NcApiCoroutines import com.nextcloud.talk.chat.data.ChatMessageRepository +import com.nextcloud.talk.chat.data.network.ChatMessageSyncer import com.nextcloud.talk.chat.data.network.ChatNetworkDataSource import com.nextcloud.talk.chat.data.network.OfflineFirstChatRepository import com.nextcloud.talk.logger.Logger @@ -139,20 +140,37 @@ class RepositoryModule { fun provideInvitationsRepository(ncApi: NcApi, ncApiCoroutines: NcApiCoroutines): InvitationsRepository = InvitationsRepositoryImpl(ncApi, ncApiCoroutines) + @Provides + @Singleton + fun provideChatMessageSyncer( + chatMessagesDao: ChatMessagesDao, + chatBlocksDao: ChatBlocksDao, + dataSource: ChatNetworkDataSource, + networkMonitor: NetworkMonitor + ): ChatMessageSyncer = + ChatMessageSyncer( + chatMessagesDao, + chatBlocksDao, + dataSource, + networkMonitor + ) + @Provides fun provideOfflineFirstChatRepository( logger: Logger, chatMessagesDao: ChatMessagesDao, chatBlocksDao: ChatBlocksDao, dataSource: ChatNetworkDataSource, - networkMonitor: NetworkMonitor + networkMonitor: NetworkMonitor, + syncer: ChatMessageSyncer ): ChatMessageRepository = OfflineFirstChatRepository( logger, chatMessagesDao, chatBlocksDao, dataSource, - networkMonitor + networkMonitor, + syncer ) @Provides diff --git a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt index aaa755a46d5..9e6e643667f 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt @@ -20,6 +20,7 @@ import com.nextcloud.talk.api.NcApiCoroutines import com.nextcloud.talk.chat.data.ChatMessageRepository import com.nextcloud.talk.chat.data.io.AudioFocusRequestManager import com.nextcloud.talk.chat.data.io.MediaRecorderManager +import com.nextcloud.talk.chat.data.network.ChatMessageSyncer import com.nextcloud.talk.chat.data.network.ChatNetworkDataSource import com.nextcloud.talk.chat.data.network.OfflineFirstChatRepository import com.nextcloud.talk.chat.data.network.RetrofitChatNetwork @@ -148,13 +149,22 @@ class ComposePreviewUtils private constructor(context: Context) { val logger: TestLogger get() = TestLogger + val chatMessageSyncer: ChatMessageSyncer + get() = ChatMessageSyncer( + chatMessagesDao, + chatBlocksDao, + chatNetworkDataSource, + networkMonitor + ) + val chatRepository: ChatMessageRepository get() = OfflineFirstChatRepository( logger, chatMessagesDao, chatBlocksDao, chatNetworkDataSource, - networkMonitor + networkMonitor, + chatMessageSyncer ) val threadsRepository: ThreadsRepository From a33133b59fec1a2df029581322ef630e929b86f5 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 2 Aug 2026 18:54:42 +0200 Subject: [PATCH 02/27] feat(chat): add room catch-up sync that creates missing chat blocks Add ChatMessageSyncer.catchUpRoom as entry point for syncing a room without an open chat: delta fetch from the newest locally known message when a chat block exists, or an initial fetch of the newest messages that creates the first chat block for never-opened rooms. Guard the chat block update against an empty persist result (conversation not yet in DB) instead of crashing. Move field map construction into the syncer. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 91 ++++++++++++++++++- .../network/OfflineFirstChatRepository.kt | 34 ++----- 2 files changed, 100 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 415d7c5bc14..427ba7a2f14 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -80,6 +80,83 @@ class ChatMessageSyncer @Inject constructor( data class SyncOutcome(val persistedNewMessages: Boolean, val newestPersistedMessageId: Long?) + /** + * Builds the query parameters for a chat pull request. [setReadMarker] stays 0 so a sync never + * moves the user's read marker. + */ + @Suppress("LongParameterList") + fun buildFieldMap( + lookIntoFuture: Boolean, + timeout: Int, + includeLastKnown: Boolean, + lastKnown: Int?, + limit: Int = DEFAULT_MESSAGES_LIMIT, + threadId: Long? = null, + lastCommonRead: Int? = null + ): HashMap { + val fieldMap = HashMap() + + fieldMap["includeLastKnown"] = if (includeLastKnown) 1 else 0 + + if (lastKnown != null) { + fieldMap["lastKnownMessageId"] = lastKnown + } + + lastCommonRead?.let { + fieldMap["lastCommonReadId"] = it + } + + threadId?.let { fieldMap["threadId"] = it.toInt() } + + fieldMap["timeout"] = timeout + fieldMap["limit"] = limit + + fieldMap["lookIntoFuture"] = if (lookIntoFuture) 1 else 0 + fieldMap["setReadMarker"] = 0 + + return fieldMap + } + + /** + * Catches up a room with the server without requiring an open chat. + * + * If a chat block exists, only the delta since the newest locally known message is fetched and + * the block is extended. For rooms without any chat block (never-opened rooms) the newest + * messages are fetched and the initial chat block is created, so cached messages become + * visible to [ChatMessagesDao] block queries right away. + */ + suspend fun catchUpRoom(target: SyncTarget, limit: Int = DEFAULT_MESSAGES_LIMIT): SyncOutcome { + if (!networkMonitor.isOnline.value) { + Log.d(TAG, "Device is offline, skipping catch-up for ${target.internalConversationId}") + return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + } + + val newestMessageIdFromDb = + chatBlocksDao.getNewestMessageIdFromChatBlocks(target.internalConversationId, target.threadId) + + val fieldMap = if (newestMessageIdFromDb > 0) { + buildFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = newestMessageIdFromDb.toInt(), + limit = limit, + threadId = target.threadId + ) + } else { + buildFieldMap( + lookIntoFuture = false, + timeout = 0, + includeLastKnown = true, + lastKnown = null, + limit = limit, + threadId = target.threadId + ) + } + + return pullAndPersistMessages(target, fieldMap) + } + fun pullMessagesFlow(target: SyncTarget, fieldMap: HashMap): Flow = flow { var attempts = 1 @@ -168,7 +245,10 @@ class ChatMessageSyncer @Inject constructor( hasHistory, events ) - return SyncOutcome(persistedNewMessages = true, newestPersistedMessageId = newestPersistedId) + return SyncOutcome( + persistedNewMessages = newestPersistedId != null, + newestPersistedMessageId = newestPersistedId + ) } else { Log.d(TAG, "No new messages to update") return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) @@ -206,6 +286,14 @@ class ChatMessageSyncer @Inject constructor( val chatMessageEntities = persistChatMessagesAndHandleSystemMessages(target, chatMessagesJson, emitOnIncoming = lookIntoFuture, events) + if (chatMessageEntities.isEmpty()) { + // Persisting was skipped because the conversation is not in the DB yet (see + // persistChatMessagesAndHandleSystemMessages). Without persisted messages there must be + // no chat block update either, otherwise a block would reference missing messages. + Log.w(TAG, "No messages were persisted for ${target.internalConversationId}, skipping chat block update") + return null + } + val oldestIdFromSync = chatMessageEntities.minByOrNull { it.id }!!.id val newestIdFromSync = chatMessageEntities.maxByOrNull { it.id }!!.id Log.d(TAG, "oldestIdFromSync: $oldestIdFromSync") @@ -481,6 +569,7 @@ class ChatMessageSyncer @Inject constructor( val NO_EVENTS: Events = object : Events {} + private const val DEFAULT_MESSAGES_LIMIT = 100 private const val HTTP_CODE_OK: Int = 200 private const val HTTP_CODE_NOT_MODIFIED = 304 private const val HTTP_CODE_PRECONDITION_FAILED = 412 diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 22ae6554672..5a87122b444 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -366,36 +366,22 @@ class OfflineFirstChatRepository @Inject constructor( } } - @Suppress("LongParameterList") private fun getFieldMap( lookIntoFuture: Boolean, timeout: Int, includeLastKnown: Boolean, lastKnown: Int?, limit: Int = DEFAULT_MESSAGES_LIMIT - ): HashMap { - val fieldMap = HashMap() - - fieldMap["includeLastKnown"] = if (includeLastKnown) 1 else 0 - - if (lastKnown != null) { - fieldMap["lastKnownMessageId"] = lastKnown - } - - newXChatLastCommonRead?.let { - fieldMap["lastCommonReadId"] = it - } - - threadId?.let { fieldMap["threadId"] = it.toInt() } - - fieldMap["timeout"] = timeout - fieldMap["limit"] = limit - - fieldMap["lookIntoFuture"] = if (lookIntoFuture) 1 else 0 - fieldMap["setReadMarker"] = 0 - - return fieldMap - } + ): HashMap = + syncer.buildFieldMap( + lookIntoFuture = lookIntoFuture, + timeout = timeout, + includeLastKnown = includeLastKnown, + lastKnown = lastKnown, + limit = limit, + threadId = threadId, + lastCommonRead = newXChatLastCommonRead + ) override suspend fun getNumberOfThreadReplies(threadId: Long): Int = chatDao.getNumberOfThreadReplies(internalConversationId, threadId) From 66a1044c7051be43e645306a6fc191b9f4209d38 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 2 Aug 2026 23:32:54 +0200 Subject: [PATCH 03/27] feat(chat): support fetching messages without clearing notifications Send markNotificationsAsRead=0 on background message fetches and gate the behavior on the chat-keep-notifications server capability, so a background sync neither moves the read marker (setReadMarker=0 is already sent) nor dismisses the user's push notifications. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 33 ++++++++++++++++--- .../nextcloud/talk/utils/CapabilitiesUtil.kt | 1 + 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 427ba7a2f14..c4968a7156c 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -19,6 +19,7 @@ import com.nextcloud.talk.data.database.model.ChatMessageEntity import com.nextcloud.talk.data.network.NetworkMonitor import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.models.json.chat.ChatMessageJson +import com.nextcloud.talk.utils.SpreedFeatures import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first @@ -81,8 +82,10 @@ class ChatMessageSyncer @Inject constructor( data class SyncOutcome(val persistedNewMessages: Boolean, val newestPersistedMessageId: Long?) /** - * Builds the query parameters for a chat pull request. [setReadMarker] stays 0 so a sync never - * moves the user's read marker. + * Builds the query parameters for a chat pull request. setReadMarker stays 0 so a sync never + * moves the user's read marker. Background fetches must additionally pass + * [markNotificationsAsRead] = false so the server keeps push notifications for the fetched + * messages — only supported when the server has the chat-keep-notifications capability. */ @Suppress("LongParameterList") fun buildFieldMap( @@ -92,7 +95,8 @@ class ChatMessageSyncer @Inject constructor( lastKnown: Int?, limit: Int = DEFAULT_MESSAGES_LIMIT, threadId: Long? = null, - lastCommonRead: Int? = null + lastCommonRead: Int? = null, + markNotificationsAsRead: Boolean = true ): HashMap { val fieldMap = HashMap() @@ -114,6 +118,10 @@ class ChatMessageSyncer @Inject constructor( fieldMap["lookIntoFuture"] = if (lookIntoFuture) 1 else 0 fieldMap["setReadMarker"] = 0 + if (!markNotificationsAsRead) { + fieldMap["markNotificationsAsRead"] = 0 + } + return fieldMap } @@ -124,6 +132,10 @@ class ChatMessageSyncer @Inject constructor( * the block is extended. For rooms without any chat block (never-opened rooms) the newest * messages are fetched and the initial chat block is created, so cached messages become * visible to [ChatMessagesDao] block queries right away. + * + * Requires the chat-keep-notifications capability: without it, a background fetch would + * dismiss the user's push notifications for the fetched messages, so the catch-up is skipped + * entirely (same guard as on iOS). */ suspend fun catchUpRoom(target: SyncTarget, limit: Int = DEFAULT_MESSAGES_LIMIT): SyncOutcome { if (!networkMonitor.isOnline.value) { @@ -131,6 +143,15 @@ class ChatMessageSyncer @Inject constructor( return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) } + if (!target.user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value)) { + Log.d( + TAG, + "Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, " + + "skipping catch-up for ${target.internalConversationId}" + ) + return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + } + val newestMessageIdFromDb = chatBlocksDao.getNewestMessageIdFromChatBlocks(target.internalConversationId, target.threadId) @@ -141,7 +162,8 @@ class ChatMessageSyncer @Inject constructor( includeLastKnown = false, lastKnown = newestMessageIdFromDb.toInt(), limit = limit, - threadId = target.threadId + threadId = target.threadId, + markNotificationsAsRead = false ) } else { buildFieldMap( @@ -150,7 +172,8 @@ class ChatMessageSyncer @Inject constructor( includeLastKnown = true, lastKnown = null, limit = limit, - threadId = target.threadId + threadId = target.threadId, + markNotificationsAsRead = false ) } diff --git a/app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt b/app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt index 6e11eed629c..ca41c1279fe 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt @@ -40,6 +40,7 @@ enum class SpreedFeatures(val value: String) { GEO_LOCATION_SHARING("geo-location-sharing"), TALK_POLLS("talk-polls"), FAVORITES("favorites"), + CHAT_KEEP_NOTIFICATIONS("chat-keep-notifications"), CHAT_READ_MARKER("chat-read-marker"), CHAT_UNREAD("chat-unread"), EDIT_MESSAGES("edit-messages"), From 49aad872f230056d2cedd00fa4e18c63c69da33e Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 2 Aug 2026 23:49:41 +0200 Subject: [PATCH 04/27] feat(conversations): prefetch unread messages during room list sync After upserting conversations from GET /room, catch up messages of rooms whose lastActivity advanced since the last sync (matching the iOS behavior) and of unread rooms that have no cached messages yet. The catch-up runs via ChatMessageSyncer.catchUpRoom after the room list was emitted, so unread messages are already in the local database when a chat is opened. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 7 +++ .../OfflineFirstConversationsRepository.kt | 60 ++++++++++++++++++- .../talk/dagger/modules/RepositoryModule.kt | 6 +- .../talk/utils/preview/ComposePreviewUtils.kt | 3 +- 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index c4968a7156c..2e6895b8c1a 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -125,6 +125,13 @@ class ChatMessageSyncer @Inject constructor( return fieldMap } + /** + * Returns true when messages of the conversation (or thread) are cached locally and covered by + * a chat block, i.e. a catch-up only needs to fetch the delta. + */ + fun hasLocalChatBlock(internalConversationId: String, threadId: Long?): Boolean = + chatBlocksDao.getNewestMessageIdFromChatBlocks(internalConversationId, threadId) > 0 + /** * Catches up a room with the server without requiring an open chat. * 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 182a2469603..a3bed000578 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 @@ -9,6 +9,7 @@ package com.nextcloud.talk.conversationlist.data.network import android.util.Log +import com.nextcloud.talk.chat.data.network.ChatMessageSyncer import com.nextcloud.talk.chat.data.network.ChatNetworkDataSource import com.nextcloud.talk.conversationlist.data.OfflineConversationsRepository import com.nextcloud.talk.data.database.dao.ConversationsDao @@ -18,7 +19,9 @@ import com.nextcloud.talk.data.database.model.ConversationEntity import com.nextcloud.talk.data.network.NetworkMonitor import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.models.domain.ConversationModel +import com.nextcloud.talk.utils.ApiUtils import com.nextcloud.talk.utils.CapabilitiesUtil.isUserStatusAvailable +import com.nextcloud.talk.utils.SpreedFeatures import io.reactivex.Observer import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.Disposable @@ -39,7 +42,8 @@ class OfflineFirstConversationsRepository @Inject constructor( private val dao: ConversationsDao, private val network: ConversationsNetworkDataSource, private val chatNetworkDataSource: ChatNetworkDataSource, - private val networkMonitor: NetworkMonitor + private val networkMonitor: NetworkMonitor, + private val chatMessageSyncer: ChatMessageSyncer ) : OfflineConversationsRepository { override val roomListFlow: Flow> get() = _roomListFlow @@ -159,17 +163,70 @@ class OfflineFirstConversationsRepository @Inject constructor( it.asEntity(user.id!!) } + val previousConversations = dao.getConversationsForUser(user.id!!).first() + .associateBy { it.internalId } + deleteLeftConversations( user, conversationsFromSync ) dao.upsertConversations(user.id!!, conversationsFromSync) + + val roomsWithNewMessages = getRoomsWithNewMessages(conversationsFromSync, previousConversations) + scope.launch { catchUpRoomsWithNewMessages(user, roomsWithNewMessages) } } catch (e: Exception) { Log.e(TAG, "Something went wrong when fetching conversations", e) } return conversationsFromSync } + /** + * Determines the rooms whose messages should be caught up in the background: rooms with + * activity newer than the last synced state (matching the iOS behavior) plus unread rooms that + * have no cached messages yet (never-opened rooms). + */ + private fun getRoomsWithNewMessages( + conversationsFromSync: List, + previousConversations: Map + ): List = + conversationsFromSync.filter { room -> + val previous = previousConversations[room.internalId] + val activityAdvanced = previous == null || room.lastActivity > previous.lastActivity + activityAdvanced || + (room.unreadMessages > 0 && !chatMessageSyncer.hasLocalChatBlock(room.internalId, null)) + } + + /** + * Prefetches the messages of [rooms] into the local database so they are instantly visible + * when a chat is opened. Runs after the room list sync; failures are logged and never affect + * the conversation list itself. + */ + private suspend fun catchUpRoomsWithNewMessages(user: User, rooms: List) { + if (rooms.isEmpty()) { + return + } + + if (!user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value)) { + Log.d(TAG, "Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, skipping message catch-up") + return + } + + val credentials = ApiUtils.getCredentials(user.username, user.token) ?: return + + Log.d(TAG, "Catching up messages for ${rooms.size} rooms") + for (room in rooms) { + val target = ChatMessageSyncer.SyncTarget( + user = user, + roomToken = room.token, + threadId = null, + credentials = credentials, + urlForChatting = ApiUtils.getUrlForChat(CHAT_API_VERSION, user.baseUrl!!, room.token) + ) + runCatching { chatMessageSyncer.catchUpRoom(target) } + .onFailure { Log.e(TAG, "Message catch-up failed for room ${room.token}", it) } + } + } + private suspend fun deleteLeftConversations(user: User, conversationsFromSync: List) { val conversationsFromSyncIds = conversationsFromSync.map { it.internalId }.toSet() val oldConversationsFromDb = dao.getConversationsForUser(user.id!!).first() @@ -193,5 +250,6 @@ class OfflineFirstConversationsRepository @Inject constructor( companion object { val TAG = OfflineFirstConversationsRepository::class.simpleName + private const val CHAT_API_VERSION = 1 } } diff --git a/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt b/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt index 773277937ff..46bd2b51731 100644 --- a/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt +++ b/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt @@ -179,13 +179,15 @@ class RepositoryModule { dao: ConversationsDao, dataSource: ConversationsNetworkDataSource, chatNetworkDataSource: ChatNetworkDataSource, - networkMonitor: NetworkMonitor + networkMonitor: NetworkMonitor, + chatMessageSyncer: ChatMessageSyncer ): OfflineConversationsRepository = OfflineFirstConversationsRepository( dao, dataSource, chatNetworkDataSource, - networkMonitor + networkMonitor, + chatMessageSyncer ) @Provides diff --git a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt index 9e6e643667f..dd7f259daad 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt @@ -178,7 +178,8 @@ class ComposePreviewUtils private constructor(context: Context) { conversationsDao, conversationNetworkDataSource, chatNetworkDataSource, - networkMonitor + networkMonitor, + chatMessageSyncer ) val reactionsRepository: ReactionsRepository From 3bfa4f7cf51d36173814c09c8e611b2c1982063d Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 2 Aug 2026 23:57:43 +0200 Subject: [PATCH 05/27] feat(conversations): guard message prefetch with caps and saver modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limit the room-list message prefetch to the 20 most recently active rooms with at most 3 concurrent requests, and skip it entirely in battery saver mode or when background data is restricted on a metered network — mirroring the Low Power Mode guard on iOS. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../OfflineFirstConversationsRepository.kt | 73 ++++++++++++++++--- .../talk/dagger/modules/RepositoryModule.kt | 6 +- .../talk/utils/preview/ComposePreviewUtils.kt | 3 +- 3 files changed, 67 insertions(+), 15 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 a3bed000578..424ceaf4891 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 @@ -8,6 +8,9 @@ package com.nextcloud.talk.conversationlist.data.network +import android.content.Context +import android.net.ConnectivityManager +import android.os.PowerManager import android.util.Log import com.nextcloud.talk.chat.data.network.ChatMessageSyncer import com.nextcloud.talk.chat.data.network.ChatNetworkDataSource @@ -33,8 +36,11 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.sync.Semaphore +import kotlinx.coroutines.sync.withPermit import javax.inject.Inject import kotlin.collections.map @@ -43,7 +49,8 @@ class OfflineFirstConversationsRepository @Inject constructor( private val network: ConversationsNetworkDataSource, private val chatNetworkDataSource: ChatNetworkDataSource, private val networkMonitor: NetworkMonitor, - private val chatMessageSyncer: ChatMessageSyncer + private val chatMessageSyncer: ChatMessageSyncer, + private val context: Context ) : OfflineConversationsRepository { override val roomListFlow: Flow> get() = _roomListFlow @@ -200,6 +207,11 @@ class OfflineFirstConversationsRepository @Inject constructor( * Prefetches the messages of [rooms] into the local database so they are instantly visible * when a chat is opened. Runs after the room list sync; failures are logged and never affect * the conversation list itself. + * + * The catch-up is skipped in battery saver mode and when background data is restricted on a + * metered network (mirroring the Low Power Mode guard on iOS), and is bounded to the + * [MAX_ROOMS_TO_CATCH_UP] most recently active rooms with [MAX_CONCURRENT_CATCH_UPS] parallel + * requests, so a fresh install with many rooms cannot cause an unbounded request burst. */ private suspend fun catchUpRoomsWithNewMessages(user: User, rooms: List) { if (rooms.isEmpty()) { @@ -211,22 +223,57 @@ class OfflineFirstConversationsRepository @Inject constructor( return } + if (isPowerSaveMode()) { + Log.d(TAG, "Battery saver is active, skipping message catch-up") + return + } + + if (isBackgroundDataRestricted()) { + Log.d(TAG, "Background data is restricted on a metered network, skipping message catch-up") + return + } + val credentials = ApiUtils.getCredentials(user.username, user.token) ?: return - Log.d(TAG, "Catching up messages for ${rooms.size} rooms") - for (room in rooms) { - val target = ChatMessageSyncer.SyncTarget( - user = user, - roomToken = room.token, - threadId = null, - credentials = credentials, - urlForChatting = ApiUtils.getUrlForChat(CHAT_API_VERSION, user.baseUrl!!, room.token) - ) - runCatching { chatMessageSyncer.catchUpRoom(target) } - .onFailure { Log.e(TAG, "Message catch-up failed for room ${room.token}", it) } + val cappedRooms = rooms + .sortedByDescending { it.lastActivity } + .take(MAX_ROOMS_TO_CATCH_UP) + if (cappedRooms.size < rooms.size) { + Log.w(TAG, "Capping message catch-up to ${cappedRooms.size} of ${rooms.size} rooms") + } + + Log.d(TAG, "Catching up messages for ${cappedRooms.size} rooms") + coroutineScope { + val semaphore = Semaphore(MAX_CONCURRENT_CATCH_UPS) + cappedRooms.forEach { room -> + launch { + semaphore.withPermit { + val target = ChatMessageSyncer.SyncTarget( + user = user, + roomToken = room.token, + threadId = null, + credentials = credentials, + urlForChatting = ApiUtils.getUrlForChat(CHAT_API_VERSION, user.baseUrl!!, room.token) + ) + runCatching { chatMessageSyncer.catchUpRoom(target) } + .onFailure { Log.e(TAG, "Message catch-up failed for room ${room.token}", it) } + } + } + } } } + private fun isPowerSaveMode(): Boolean { + val powerManager = context.getSystemService(Context.POWER_SERVICE) as PowerManager + return powerManager.isPowerSaveMode + } + + private fun isBackgroundDataRestricted(): Boolean { + val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager + return connectivityManager.isActiveNetworkMetered && + connectivityManager.restrictBackgroundStatus == ConnectivityManager.RESTRICT_BACKGROUND_STATUS_ENABLED + } + private suspend fun deleteLeftConversations(user: User, conversationsFromSync: List) { val conversationsFromSyncIds = conversationsFromSync.map { it.internalId }.toSet() val oldConversationsFromDb = dao.getConversationsForUser(user.id!!).first() @@ -251,5 +298,7 @@ class OfflineFirstConversationsRepository @Inject constructor( companion object { val TAG = OfflineFirstConversationsRepository::class.simpleName private const val CHAT_API_VERSION = 1 + private const val MAX_ROOMS_TO_CATCH_UP = 20 + private const val MAX_CONCURRENT_CATCH_UPS = 3 } } diff --git a/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt b/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt index 46bd2b51731..b17361c89ad 100644 --- a/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt +++ b/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt @@ -180,14 +180,16 @@ class RepositoryModule { dataSource: ConversationsNetworkDataSource, chatNetworkDataSource: ChatNetworkDataSource, networkMonitor: NetworkMonitor, - chatMessageSyncer: ChatMessageSyncer + chatMessageSyncer: ChatMessageSyncer, + context: Context ): OfflineConversationsRepository = OfflineFirstConversationsRepository( dao, dataSource, chatNetworkDataSource, networkMonitor, - chatMessageSyncer + chatMessageSyncer, + context ) @Provides diff --git a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt index dd7f259daad..0da2a13a4c6 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtils.kt @@ -179,7 +179,8 @@ class ComposePreviewUtils private constructor(context: Context) { conversationNetworkDataSource, chatNetworkDataSource, networkMonitor, - chatMessageSyncer + chatMessageSyncer, + mContext ) val reactionsRepository: ReactionsRepository From 2ede0ae3e5486251f59ee730186760b1e909f5a8 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 3 Aug 2026 00:08:47 +0200 Subject: [PATCH 06/27] fix(chat): trust local cache on chat open and fetch only the delta Skip the initial newest-100 request when cached messages already reach the conversation's lastMessage.id, and replace the forced full fetch on chat-relay servers with a delta fetch from the newest cached message. This keeps the relay path's backlog guarantee while making chat open network-free after a successful prefetch. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../network/OfflineFirstChatRepository.kt | 101 ++++++++++++------ 1 file changed, 68 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 5a87122b444..b7a73b84484 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -189,53 +189,88 @@ class OfflineFirstChatRepository @Inject constructor( val weAlreadyHaveSomeOfflineMessages = newestMessageIdFromDb > 0 val weHaveAtLeastTheLastReadMessage = newestMessageIdFromDb >= conversationModel.lastReadMessage.toLong() + val lastMessageIdFromServer = conversationModel.lastMessage?.id ?: 0 + val weHaveTheLastMessage = newestMessageIdFromDb >= lastMessageIdFromServer Log.d(TAG, "weAlreadyHaveSomeOfflineMessages:$weAlreadyHaveSomeOfflineMessages") Log.d(TAG, "weHaveAtLeastTheLastReadMessage:$weHaveAtLeastTheLastReadMessage") + Log.d(TAG, "weHaveTheLastMessage:$weHaveTheLastMessage (lastMessageIdFromServer:$lastMessageIdFromServer)") Log.d(TAG, "isChatRelaySupported:$isChatRelaySupported") - if (weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage && !isChatRelaySupported) { - Log.d( - TAG, - "Initial online request is skipped because offline messages are up to date" + - " until lastReadMessage" - ) + when { + weAlreadyHaveSomeOfflineMessages && weHaveTheLastMessage -> { + // The offline messages already reach the conversation's last message (e.g. because + // the room list sync prefetched them), so no initial request is needed at all — + // regardless of chat relay. Anything newer is handled by long polling, the chat + // relay or the insurance requests. + Log.d( + TAG, + "Initial online request is skipped because offline messages are up to date" + + " until the conversation's last message" + ) + } - // For messages newer than lastRead, lookIntoFuture will load them. - // We must only end up here when NO HPB is used! - // If a HPB is used, longPolling is not available to handle loading of newer messages. - // When a HPB is used the initial request must be made. - } else { - if (isChatRelaySupported) { + weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage && !isChatRelaySupported -> { Log.d( TAG, - "An online request for newest 100 messages is made because chatRelay is supported (No long " + - "polling available to catch up with messages newer than last read.)" + "Initial online request is skipped because offline messages are up to date" + + " until lastReadMessage" ) - } else if (!weAlreadyHaveSomeOfflineMessages) { - Log.d(TAG, "An online request for newest 100 messages is made because offline chat is empty") - if (networkMonitor.isOnline.value.not()) { - // _generalUIFlow.emit(ChatActivity.NO_OFFLINE_MESSAGES_FOUND) - } - } else { + + // For messages newer than lastRead, lookIntoFuture will load them. + // We must only end up here when NO HPB is used! + // If a HPB is used, longPolling is not available to handle loading of newer messages. + } + + weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage && isChatRelaySupported -> { + // The chat relay only pushes messages that arrive while being connected, so the + // backlog since the newest offline message must be closed with an initial request. + // A delta fetch is enough — no need to re-download the newest 100 messages. Log.d( TAG, - "An online request for newest 100 messages is made because we don't have the lastReadMessage " + - "(gaps could be closed by scrolling up to merge the chatblocks)" + "A delta request from the newest offline message is made because chatRelay is" + + " supported (the relay cannot deliver messages that arrived while the app was closed)" ) + + val fieldMap = getFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = newestMessageIdFromDb.toInt() + ) + withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) + withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token) + + Log.d(TAG, "Starting delta request for initial loading") + getAndPersistMessages(withNetworkParams) } - // set up field map to load the newest messages - val fieldMap = getFieldMap( - lookIntoFuture = false, - timeout = 0, - includeLastKnown = true, - lastKnown = null - ) - withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) - withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token) + else -> { + if (!weAlreadyHaveSomeOfflineMessages) { + Log.d(TAG, "An online request for newest 100 messages is made because offline chat is empty") + if (networkMonitor.isOnline.value.not()) { + // _generalUIFlow.emit(ChatActivity.NO_OFFLINE_MESSAGES_FOUND) + } + } else { + Log.d( + TAG, + "An online request for newest 100 messages is made because we don't have the " + + "lastReadMessage (gaps could be closed by scrolling up to merge the chatblocks)" + ) + } - Log.d(TAG, "Starting online request for initial loading") - getAndPersistMessages(withNetworkParams) + // set up field map to load the newest messages + val fieldMap = getFieldMap( + lookIntoFuture = false, + timeout = 0, + includeLastKnown = true, + lastKnown = null + ) + withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) + withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token) + + Log.d(TAG, "Starting online request for initial loading") + getAndPersistMessages(withNetworkParams) + } } } From 12b296f6f9aa94c31d3d54768810f4a6297411de Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 3 Aug 2026 00:13:48 +0200 Subject: [PATCH 07/27] fix(chat): do not block initial message load on websocket connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Start loading messages immediately when a chat is opened and decide the live-update mode (chat relay vs long polling) in a parallel coroutine once the websocket state is known. The backlog delta fetch is now made regardless of the mode — required for chat relay, and on long-polling servers it only front-loads what the first poll request would have fetched — so loadInitialMessages no longer needs to know about chat relay at all. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../talk/chat/data/ChatMessageRepository.kt | 2 +- .../network/OfflineFirstChatRepository.kt | 36 ++++++------------- .../talk/chat/viewmodels/ChatViewModel.kt | 23 ++++++------ 3 files changed, 23 insertions(+), 38 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt index 9a55fac9bd9..5cbb1fb120d 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt @@ -58,7 +58,7 @@ interface ChatMessageRepository : LifecycleAwareManager { fun updateConversation(conversationModel: ConversationModel) - suspend fun loadInitialMessages(withNetworkParams: Bundle, isChatRelaySupported: Boolean) + suspend fun loadInitialMessages(withNetworkParams: Bundle) suspend fun startMessagePolling(hasHighPerformanceBackend: Boolean) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index b7a73b84484..e2171cbc214 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -175,7 +175,7 @@ class OfflineFirstChatRepository @Inject constructor( } } - override suspend fun loadInitialMessages(withNetworkParams: Bundle, isChatRelaySupported: Boolean) { + override suspend fun loadInitialMessages(withNetworkParams: Bundle) { logger.d(TAG, "---- loadInitialMessages ------------") cleanupExpiredMessages() newXChatLastCommonRead = conversationModel.lastCommonReadMessage @@ -194,14 +194,13 @@ class OfflineFirstChatRepository @Inject constructor( Log.d(TAG, "weAlreadyHaveSomeOfflineMessages:$weAlreadyHaveSomeOfflineMessages") Log.d(TAG, "weHaveAtLeastTheLastReadMessage:$weHaveAtLeastTheLastReadMessage") Log.d(TAG, "weHaveTheLastMessage:$weHaveTheLastMessage (lastMessageIdFromServer:$lastMessageIdFromServer)") - Log.d(TAG, "isChatRelaySupported:$isChatRelaySupported") when { weAlreadyHaveSomeOfflineMessages && weHaveTheLastMessage -> { // The offline messages already reach the conversation's last message (e.g. because // the room list sync prefetched them), so no initial request is needed at all — - // regardless of chat relay. Anything newer is handled by long polling, the chat - // relay or the insurance requests. + // regardless of the live-update mode. Anything newer is handled by long polling, + // the chat relay or the insurance requests. Log.d( TAG, "Initial online request is skipped because offline messages are up to date" + @@ -209,27 +208,14 @@ class OfflineFirstChatRepository @Inject constructor( ) } - weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage && !isChatRelaySupported -> { - Log.d( - TAG, - "Initial online request is skipped because offline messages are up to date" + - " until lastReadMessage" - ) - - // For messages newer than lastRead, lookIntoFuture will load them. - // We must only end up here when NO HPB is used! - // If a HPB is used, longPolling is not available to handle loading of newer messages. - } - - weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage && isChatRelaySupported -> { - // The chat relay only pushes messages that arrive while being connected, so the - // backlog since the newest offline message must be closed with an initial request. - // A delta fetch is enough — no need to re-download the newest 100 messages. - Log.d( - TAG, - "A delta request from the newest offline message is made because chatRelay is" + - " supported (the relay cannot deliver messages that arrived while the app was closed)" - ) + weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage -> { + // Close the backlog since the newest offline message with a delta fetch. This is + // required on chat-relay servers (the relay cannot deliver messages that arrived + // while the app was closed) and is equally cheap on long-polling servers, where it + // just front-loads what the first poll request would have fetched. This way the + // initial load never has to know the live-update mode, i.e. it must not wait for + // the websocket. + Log.d(TAG, "A delta request from the newest offline message is made to close the backlog") val fieldMap = getFieldMap( lookIntoFuture = true, diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index c71dc15d05d..e5b7e8978e5 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -1277,20 +1277,20 @@ class ChatViewModel @AssistedInject constructor( chatRepository.updateConversation(conversation) - val isChatRelaySupported = withTimeoutOrNull(WEBSOCKET_CONNECT_TIMEOUT_MS) { - awaitChatRelaySupport(user) - } ?: false + // The live-update mode (chat relay vs long polling) is decided in parallel: the + // initial load must never wait for the websocket to connect. + viewModelScope.launch { + val isChatRelaySupported = withTimeoutOrNull(WEBSOCKET_CONNECT_TIMEOUT_MS) { + awaitChatRelaySupport(user) + } ?: false + startMessagePolling(isChatRelaySupported) + } loadInitialMessages( withCredentials = credentials, - withUrl = url, - isChatRelaySupported = isChatRelaySupported + withUrl = url ) - viewModelScope.launch { - startMessagePolling(isChatRelaySupported) - } - getCapabilities(user, chatRoomToken, conversation) } .launchIn(viewModelScope) @@ -1712,13 +1712,12 @@ class ChatViewModel @AssistedInject constructor( } } - suspend fun loadInitialMessages(withCredentials: String, withUrl: String, isChatRelaySupported: Boolean) { + suspend fun loadInitialMessages(withCredentials: String, withUrl: String) { val bundle = Bundle() bundle.putString(BundleKeys.KEY_CHAT_URL, withUrl) bundle.putString(BundleKeys.KEY_CREDENTIALS, withCredentials) chatRepository.loadInitialMessages( - withNetworkParams = bundle, - isChatRelaySupported = isChatRelaySupported + withNetworkParams = bundle ) } From 534f2d65ae8cb528a42a06962370454d12003f2a Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 3 Aug 2026 00:27:08 +0200 Subject: [PATCH 08/27] feat(notifications): prefetch chat messages on push receipt After displaying a message notification, trigger a single-room catch-up so the pushed message and any backlog are persisted to the local database while the app is backgrounded. Best effort only: failures never delay or suppress the notification. Skipped without the chat-keep-notifications capability or in battery saver. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../nextcloud/talk/jobs/NotificationWorker.kt | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt index 5256272b7a3..835ec532313 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt @@ -20,6 +20,7 @@ import android.os.Build import android.os.Bundle import android.os.Handler import android.os.Looper +import android.os.PowerManager import android.os.SystemClock import android.service.notification.StatusBarNotification import android.text.TextUtils @@ -52,6 +53,7 @@ import com.nextcloud.talk.application.NextcloudTalkApplication import com.nextcloud.talk.application.NextcloudTalkApplication.Companion.sharedApplication import com.nextcloud.talk.arbitrarystorage.ArbitraryStorageManager import com.nextcloud.talk.callnotification.CallNotificationActivity +import com.nextcloud.talk.chat.data.network.ChatMessageSyncer import com.nextcloud.talk.chat.data.network.ChatNetworkDataSource import com.nextcloud.talk.conversationlist.DirectShareHelper import com.nextcloud.talk.data.user.model.User @@ -136,6 +138,9 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor var chatNetworkDataSource: ChatNetworkDataSource? = null @Inject set + var chatMessageSyncer: ChatMessageSyncer? = null + @Inject set + @Inject lateinit var userManager: UserManager @@ -211,6 +216,48 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor private fun handleNonCallPushMessage() { val mainActivityIntent = createMainActivityIntent() getNcDataAndShowNotification(mainActivityIntent) + if (pushMessage.type == TYPE_CHAT) { + catchUpPushedRoom() + } + } + + /** + * Prefetches the pushed room's messages into the local database so they are instantly visible + * when the chat is opened from the notification (or later). Best effort only: failures are + * logged and never delay or suppress the notification, which is displayed independently. + * Skipped in battery saver mode; the chat-keep-notifications capability gate and the offline + * check are handled inside [ChatMessageSyncer.catchUpRoom]. + */ + private fun catchUpPushedRoom() { + val roomToken = pushMessage.id ?: return + val syncer = chatMessageSyncer ?: return + + if (isPowerSaveMode()) { + logger.d(TAG, "Battery saver is active, skipping message catch-up for pushed room") + return + } + + // the user from the push signature verification may carry stale capabilities, so resolve + // the current state before the capability check in catchUpRoom + val currentUser = userManager.getUserWithId(user.id!!).blockingGet() ?: return + + val target = ChatMessageSyncer.SyncTarget( + user = currentUser, + roomToken = roomToken, + threadId = null, + credentials = credentials, + urlForChatting = ApiUtils.getUrlForChat(CHAT_API_VERSION, currentUser.baseUrl!!, roomToken) + ) + runCatching { + runBlocking { syncer.catchUpRoom(target) } + }.onFailure { + Log.e(TAG, "Message catch-up after push failed for room $roomToken", it) + } + } + + private fun isPowerSaveMode(): Boolean { + val powerManager = applicationContext.getSystemService(Context.POWER_SERVICE) as PowerManager + return powerManager.isPowerSaveMode } private fun handleRemoteTalkSharePushMessage() { @@ -1201,6 +1248,7 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor companion object { val TAG: String = NotificationWorker::class.java.simpleName private const val TYPE_CHAT = "chat" + private const val CHAT_API_VERSION = 1 private const val TYPE_ROOM = "room" private const val TYPE_CALL = "call" private const val TYPE_RECORDING = "recording" From 2df1d00f5d46d47335bba94a4a12cd10134bce12 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 3 Aug 2026 00:31:11 +0200 Subject: [PATCH 09/27] fix(conversations): guard conversation deletion against empty sync Skip deleteLeftConversations when GET /room unexpectedly returns no conversations while some exist locally. A broken or partial server response would otherwise delete every local conversation and, via foreign key cascade, wipe the cached chat messages and chat blocks that the message prefetch relies on. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../OfflineFirstConversationsRepository.kt | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) 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 424ceaf4891..115d7a64913 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 @@ -275,9 +275,23 @@ class OfflineFirstConversationsRepository @Inject constructor( } private suspend fun deleteLeftConversations(user: User, conversationsFromSync: List) { - val conversationsFromSyncIds = conversationsFromSync.map { it.internalId }.toSet() val oldConversationsFromDb = dao.getConversationsForUser(user.id!!).first() + if (conversationsFromSync.isEmpty() && oldConversationsFromDb.isNotEmpty()) { + // A sync that suddenly contains no conversations at all is most likely a broken or + // partial server response. Deleting the local conversations in that case would also + // wipe their cached chat messages and chat blocks via foreign key cascade, destroying + // the offline cache. Skip and let a later successful sync reconcile. + Log.w( + TAG, + "Sync returned no conversations while ${oldConversationsFromDb.size} exist locally, " + + "skipping deletion of left conversations" + ) + return + } + + val conversationsFromSyncIds = conversationsFromSync.map { it.internalId }.toSet() + val conversationIdsToDelete = oldConversationsFromDb .map { it.internalId } .filterNot { it in conversationsFromSyncIds } From 4d5f4b48751e17887a1d9db7ac5882c0162d88db Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 3 Aug 2026 00:54:40 +0200 Subject: [PATCH 10/27] fix(chat): reconcile chat blocks when expiring messages Trim chat block boundaries to the oldest/newest message that still exists after deleteExpiredMessages and delete blocks whose messages are all gone, so block boundaries never point to rows that no longer exist. The cleanup moved into ChatMessageSyncer so future background callers share it. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 47 +++++++++++++++++++ .../network/OfflineFirstChatRepository.kt | 5 +- .../talk/data/database/dao/ChatBlocksDao.kt | 10 ++++ .../talk/data/database/dao/ChatMessagesDao.kt | 34 ++++++++++++++ 4 files changed, 92 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 2e6895b8c1a..7b45d7bbd77 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -132,6 +132,52 @@ class ChatMessageSyncer @Inject constructor( fun hasLocalChatBlock(internalConversationId: String, threadId: Long?): Boolean = chatBlocksDao.getNewestMessageIdFromChatBlocks(internalConversationId, threadId) > 0 + /** + * Deletes the expired messages of a conversation and reconciles its chat blocks afterwards: + * block boundaries are trimmed to the oldest/newest message that still exists and blocks whose + * messages are all gone are deleted. Without the reconciliation, block boundaries would point + * to deleted messages, e.g. faking coverage up to a message that is no longer cached. + */ + suspend fun cleanupExpiredMessages(internalConversationId: String) { + val deletedMessages = chatDao.deleteExpiredMessages( + internalConversationId, + System.currentTimeMillis() / MILLIS_PER_SECOND + ) + if (deletedMessages == 0) { + return + } + Log.d(TAG, "Deleted $deletedMessages expired messages for $internalConversationId, reconciling chat blocks") + + val blocks = chatBlocksDao.getChatBlocksForConversation(internalConversationId) + for (block in blocks) { + val newestExistingId = chatDao.getNewestMessageIdInRange( + internalConversationId = internalConversationId, + threadId = block.threadId, + oldestMessageId = block.oldestMessageId, + newestMessageId = block.newestMessageId + ) + + if (newestExistingId == null) { + Log.d(TAG, "Deleting chat block without any remaining messages ($internalConversationId)") + chatBlocksDao.deleteChatBlocks(listOf(block)) + continue + } + + val oldestExistingId = chatDao.getOldestMessageIdInRange( + internalConversationId = internalConversationId, + threadId = block.threadId, + oldestMessageId = block.oldestMessageId, + newestMessageId = block.newestMessageId + ) ?: continue + + if (block.oldestMessageId != oldestExistingId || block.newestMessageId != newestExistingId) { + block.oldestMessageId = oldestExistingId + block.newestMessageId = newestExistingId + chatBlocksDao.upsertChatBlock(block) + } + } + } + /** * Catches up a room with the server without requiring an open chat. * @@ -600,6 +646,7 @@ class ChatMessageSyncer @Inject constructor( val NO_EVENTS: Events = object : Events {} private const val DEFAULT_MESSAGES_LIMIT = 100 + private const val MILLIS_PER_SECOND = 1000L private const val HTTP_CODE_OK: Int = 200 private const val HTTP_CODE_NOT_MODIFIED = 304 private const val HTTP_CODE_PRECONDITION_FAILED = 412 diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index e2171cbc214..8a55e64b001 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -330,10 +330,7 @@ class OfflineFirstChatRepository @Inject constructor( } private suspend fun cleanupExpiredMessages() { - // For now, only the messages are deleted without adapting the chatBlocks. It may turn out that there must be - // solutions to delete empty chatBlocks and trim chatBlocks if a first or last messages of it does not exist - // anymore - chatDao.deleteExpiredMessages(internalConversationId, System.currentTimeMillis() / MILLIES) + syncer.cleanupExpiredMessages(internalConversationId) } /** diff --git a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatBlocksDao.kt b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatBlocksDao.kt index 313ee4d3ccb..9657f769339 100644 --- a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatBlocksDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatBlocksDao.kt @@ -121,4 +121,14 @@ interface ChatBlocksDao { """ ) fun getLatestChatBlock(internalConversationId: String, threadId: Long?): Flow + + @Query( + """ + SELECT * + FROM ChatBlocks + WHERE internalConversationId = :internalConversationId + ORDER BY newestMessageId ASC + """ + ) + suspend fun getChatBlocksForConversation(internalConversationId: String): List } diff --git a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt index 9c7a289f0fa..66a743da2bf 100644 --- a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt @@ -297,6 +297,40 @@ interface ChatMessagesDao { ) suspend fun deleteExpiredMessages(internalConversationId: String, currentTimeSecs: Long): Int + @Query( + """ + SELECT MIN(id) + FROM ChatMessages + WHERE internalConversationId = :internalConversationId + AND isTemporary = 0 + AND (:threadId IS NULL OR threadId = :threadId) + AND id BETWEEN :oldestMessageId AND :newestMessageId + """ + ) + suspend fun getOldestMessageIdInRange( + internalConversationId: String, + threadId: Long?, + oldestMessageId: Long, + newestMessageId: Long + ): Long? + + @Query( + """ + SELECT MAX(id) + FROM ChatMessages + WHERE internalConversationId = :internalConversationId + AND isTemporary = 0 + AND (:threadId IS NULL OR threadId = :threadId) + AND id BETWEEN :oldestMessageId AND :newestMessageId + """ + ) + suspend fun getNewestMessageIdInRange( + internalConversationId: String, + threadId: Long?, + oldestMessageId: Long, + newestMessageId: Long + ): Long? + @Query( """ DELETE FROM chatmessages From 7c4c370b9f5ad599b1c3cbe47265615b0117010d Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 3 Aug 2026 01:02:29 +0200 Subject: [PATCH 11/27] fix(chat): derive insurance fetches from the chat blocks Replace the in-memory latestKnownMessageIdFromSync with the newest message id from the chat blocks. The field lived in the unscoped repository and was reset to zero on every chat open, so an insurance request or signaling-triggered refresh running before the first successful sync of the session queried with lastKnownMessageId=0. The database is always at least as fresh because messages and chat blocks are persisted together. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../data/network/OfflineFirstChatRepository.kt | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 8a55e64b001..ca9bd874b15 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -123,8 +123,6 @@ class OfflineFirstChatRepository @Inject constructor( private lateinit var urlForChatting: String private var threadId: Long? = null - private var latestKnownMessageIdFromSync: Long = 0 - private val requestedParentIds = mutableSetOf() override fun initData( @@ -323,7 +321,7 @@ class OfflineFirstChatRepository @Inject constructor( while (true) { delay(INSURANCE_REQUEST_DELAY) - Log.d(TAG, "execute insurance request with latestKnownMessageIdFromSync: $latestKnownMessageIdFromSync") + Log.d(TAG, "execute insurance request") fetchNewMessages() } @@ -334,17 +332,24 @@ class OfflineFirstChatRepository @Inject constructor( } /** - * Fetches messages newer than latest known message. + * Fetches messages newer than the newest message covered by the chat blocks. + * + * The anchor is read from the database instead of in-memory state: messages and chat blocks + * are persisted together, so the blocks are always at least as fresh — and unlike a field in + * this (unscoped, per-chat-open) repository they survive reopening the chat. * * @return `true` if at least one new message was received and persisted. */ override suspend fun fetchNewMessages(): Boolean { cleanupExpiredMessages() + + val newestMessageIdFromDb = chatBlocksDao.getNewestMessageIdFromChatBlocks(internalConversationId, threadId) + val fieldMap = getFieldMap( lookIntoFuture = true, timeout = 0, includeLastKnown = false, - lastKnown = latestKnownMessageIdFromSync.toInt(), + lastKnown = newestMessageIdFromDb.toInt(), limit = 200 ) val networkParams = Bundle() @@ -510,9 +515,6 @@ class OfflineFirstChatRepository @Inject constructor( private suspend fun getAndPersistMessages(bundle: Bundle): Boolean { val fieldMap = bundle.getSerializable(BundleKeys.KEY_FIELD_MAP) as HashMap val outcome = syncer.pullAndPersistMessages(syncTarget, fieldMap, syncEvents) - outcome.newestPersistedMessageId?.let { - latestKnownMessageIdFromSync = maxOf(latestKnownMessageIdFromSync, it) - } return outcome.persistedNewMessages } From c09a639ee4f6447fbc0d0d424f2c5cd9a4ddf0e7 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 3 Aug 2026 01:17:28 +0200 Subject: [PATCH 12/27] test(chat): cover message sync component Add unit tests for ChatMessageSyncer: field map safety flags, offline and capability gates of catchUpRoom, delta fetch for rooms with a chat block, initial fetch with block creation for never-opened rooms, merging of connected chat blocks, the not-modified and constraint violation paths, and chat block reconciliation after message expiry. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 12 +- .../network/OfflineFirstChatRepository.kt | 1 + .../talk/dagger/modules/RepositoryModule.kt | 2 + .../utils/preview/ComposePreviewUtilsDaos.kt | 17 + .../data/network/ChatMessageSyncerTest.kt | 343 ++++++++++++++++++ .../RoomListMessagePrefetchIntegrationTest.kt | 196 ++++++++++ 6 files changed, 569 insertions(+), 2 deletions(-) create mode 100644 app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt create mode 100644 app/src/test/java/com/nextcloud/talk/conversationlist/data/network/RoomListMessagePrefetchIntegrationTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 7b45d7bbd77..0a0ba78d86a 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -36,6 +36,7 @@ import javax.inject.Inject * account, room and thread to sync, so it can be used for any room at any time — no open chat * required. UI-bound side effects are reported through the optional [Events] listener. */ +@Suppress("TooManyFunctions") class ChatMessageSyncer @Inject constructor( private val chatDao: ChatMessagesDao, private val chatBlocksDao: ChatBlocksDao, @@ -274,6 +275,7 @@ class ChatMessageSyncer @Inject constructor( * Pulls messages from the server as described by [fieldMap], persists them and updates the * chat blocks of [target]. */ + @Suppress("LongMethod") suspend fun pullAndPersistMessages( target: SyncTarget, fieldMap: HashMap, @@ -351,6 +353,7 @@ class ChatMessageSyncer @Inject constructor( } } + @Suppress("LongParameterList") private suspend fun updateMessagesData( target: SyncTarget, chatMessagesJson: List, @@ -359,8 +362,12 @@ class ChatMessageSyncer @Inject constructor( hasHistory: Boolean, events: Events ): Long? { - val chatMessageEntities = - persistChatMessagesAndHandleSystemMessages(target, chatMessagesJson, emitOnIncoming = lookIntoFuture, events) + val chatMessageEntities = persistChatMessagesAndHandleSystemMessages( + target, + chatMessagesJson, + emitOnIncoming = lookIntoFuture, + events = events + ) if (chatMessageEntities.isEmpty()) { // Persisting was skipped because the conversation is not in the DB yet (see @@ -533,6 +540,7 @@ class ChatMessageSyncer @Inject constructor( * This handles the race condition where signaling may arrive before or after * ReactionsRepositoryImpl has written the optimistic local update. */ + @Suppress("NestedBlockDepth") private fun deriveReactionsSelf( target: SyncTarget, systemMessageJson: ChatMessageJson, diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index ca9bd874b15..d63f317bb7e 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -173,6 +173,7 @@ class OfflineFirstChatRepository @Inject constructor( } } + @Suppress("LongMethod") override suspend fun loadInitialMessages(withNetworkParams: Bundle) { logger.d(TAG, "---- loadInitialMessages ------------") cleanupExpiredMessages() diff --git a/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt b/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt index b17361c89ad..800315dfea5 100644 --- a/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt +++ b/app/src/main/java/com/nextcloud/talk/dagger/modules/RepositoryModule.kt @@ -156,6 +156,7 @@ class RepositoryModule { ) @Provides + @Suppress("LongParameterList") fun provideOfflineFirstChatRepository( logger: Logger, chatMessagesDao: ChatMessagesDao, @@ -175,6 +176,7 @@ class RepositoryModule { @Provides @Singleton + @Suppress("LongParameterList") fun provideOfflineFirstConversationsRepository( dao: ConversationsDao, dataSource: ConversationsNetworkDataSource, 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 47d621e6318..950e19bea52 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 @@ -134,6 +134,20 @@ class DummyChatMessagesDaoImpl : ChatMessagesDao { override suspend fun deleteExpiredMessages(internalConversationId: String, currentTimeSecs: Long): Int = 0 + override suspend fun getOldestMessageIdInRange( + internalConversationId: String, + threadId: Long?, + oldestMessageId: Long, + newestMessageId: Long + ): Long? = null + + override suspend fun getNewestMessageIdInRange( + internalConversationId: String, + threadId: Long?, + oldestMessageId: Long, + newestMessageId: Long + ): Long? = null + override fun getNumberOfThreadReplies(internalConversationId: String, threadId: Long): Int = 0 } @@ -288,4 +302,7 @@ class DummyChatBlocksDaoImpl : ChatBlocksDao { } override fun getLatestChatBlock(internalConversationId: String, threadId: Long?): Flow = flowOf() + + override suspend fun getChatBlocksForConversation(internalConversationId: String): List = + emptyList() } diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt new file mode 100644 index 00000000000..720b4de680d --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -0,0 +1,343 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Andy Scherzinger + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.chat.data.network + +import android.database.sqlite.SQLiteConstraintException +import com.nextcloud.talk.chat.data.model.ChatMessage +import com.nextcloud.talk.data.database.dao.ChatBlocksDao +import com.nextcloud.talk.data.database.dao.ChatMessagesDao +import com.nextcloud.talk.data.database.model.ChatBlockEntity +import com.nextcloud.talk.data.network.NetworkMonitor +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.models.json.capabilities.Capabilities +import com.nextcloud.talk.models.json.capabilities.SpreedCapability +import com.nextcloud.talk.models.json.chat.ChatMessageJson +import com.nextcloud.talk.models.json.chat.ChatOCS +import com.nextcloud.talk.models.json.chat.ChatOverall +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyBlocking +import org.mockito.kotlin.verifyNoInteractions +import org.mockito.kotlin.whenever +import org.mockito.kotlin.wheneverBlocking +import retrofit2.Response + +@Suppress("TooManyFunctions") +class ChatMessageSyncerTest { + + private val chatDao: ChatMessagesDao = mock() + private val chatBlocksDao: ChatBlocksDao = mock() + private val network: ChatNetworkDataSource = mock() + private val networkMonitor: NetworkMonitor = mock() + + private lateinit var syncer: ChatMessageSyncer + + @Before + fun setUp() { + syncer = ChatMessageSyncer(chatDao, chatBlocksDao, network, networkMonitor) + whenever(networkMonitor.isOnline).thenReturn(MutableStateFlow(true)) + } + + @Test + fun `buildFieldMap keeps push notifications when markNotificationsAsRead is false`() { + val fieldMap = syncer.buildFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = 42, + markNotificationsAsRead = false + ) + + assertEquals(0, fieldMap["markNotificationsAsRead"]) + assertEquals(0, fieldMap["setReadMarker"]) + assertEquals(1, fieldMap["lookIntoFuture"]) + assertEquals(42, fieldMap["lastKnownMessageId"]) + } + + @Test + fun `buildFieldMap omits markNotificationsAsRead by default`() { + val fieldMap = syncer.buildFieldMap( + lookIntoFuture = false, + timeout = 0, + includeLastKnown = true, + lastKnown = null + ) + + assertFalse(fieldMap.containsKey("markNotificationsAsRead")) + assertFalse(fieldMap.containsKey("lastKnownMessageId")) + assertEquals(0, fieldMap["setReadMarker"]) + assertEquals(1, fieldMap["includeLastKnown"]) + } + + @Test + fun `catchUpRoom skips when offline`() = + runTest { + whenever(networkMonitor.isOnline).thenReturn(MutableStateFlow(false)) + + val outcome = syncer.catchUpRoom(target()) + + assertFalse(outcome.persistedNewMessages) + verifyNoInteractions(network) + } + + @Test + fun `catchUpRoom skips without chat-keep-notifications capability`() = + runTest { + val outcome = syncer.catchUpRoom(target(user(withKeepNotificationsCapability = false))) + + assertFalse(outcome.persistedNewMessages) + verifyNoInteractions(network) + } + + @Test + fun `catchUpRoom delta-fetches from the newest cached message when a chat block exists`() = + runTest { + val existingBlock = block(oldest = 10, newest = 42) + whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null)) + .thenReturn(42L) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(listOf(existingBlock))) + whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) + .thenReturn(flowOf(listOf(existingBlock))) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(43), message(44)))) + + val outcome = syncer.catchUpRoom(target()) + + assertTrue(outcome.persistedNewMessages) + assertEquals(44L, outcome.newestPersistedMessageId) + + val fieldMapCaptor = argumentCaptor>() + verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) } + assertEquals(1, fieldMapCaptor.firstValue["lookIntoFuture"]) + assertEquals(42, fieldMapCaptor.firstValue["lastKnownMessageId"]) + assertEquals(0, fieldMapCaptor.firstValue["markNotificationsAsRead"]) + + val blockCaptor = argumentCaptor() + verifyBlocking(chatBlocksDao) { upsertChatBlock(blockCaptor.capture()) } + assertEquals(10L, blockCaptor.firstValue.oldestMessageId) + assertEquals(44L, blockCaptor.firstValue.newestMessageId) + } + + @Test + fun `catchUpRoom fetches the newest messages and creates the first block for a never-opened room`() = + runTest { + whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null)) + .thenReturn(0L) + whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) + .thenReturn(flowOf(emptyList())) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(1), message(2), message(3)))) + + val outcome = syncer.catchUpRoom(target()) + + assertTrue(outcome.persistedNewMessages) + assertEquals(3L, outcome.newestPersistedMessageId) + + val fieldMapCaptor = argumentCaptor>() + verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) } + assertEquals(0, fieldMapCaptor.firstValue["lookIntoFuture"]) + assertEquals(1, fieldMapCaptor.firstValue["includeLastKnown"]) + assertFalse(fieldMapCaptor.firstValue.containsKey("lastKnownMessageId")) + assertEquals(0, fieldMapCaptor.firstValue["markNotificationsAsRead"]) + + val blockCaptor = argumentCaptor() + verifyBlocking(chatBlocksDao) { upsertChatBlock(blockCaptor.capture()) } + assertEquals(1L, blockCaptor.firstValue.oldestMessageId) + assertEquals(3L, blockCaptor.firstValue.newestMessageId) + } + + @Test + fun `pullAndPersistMessages merges connected chat blocks`() = + runTest { + val connectedBlocks = listOf(block(oldest = 1, newest = 5), block(oldest = 3, newest = 44)) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(listOf(connectedBlocks[1]))) + whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) + .thenReturn(flowOf(connectedBlocks)) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(43), message(44)))) + + val fieldMap = syncer.buildFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = 42 + ) + syncer.pullAndPersistMessages(target(), fieldMap) + + val mergedCaptor = argumentCaptor() + verifyBlocking(chatBlocksDao) { replaceConnectedChatBlocks(eq(connectedBlocks), mergedCaptor.capture()) } + assertEquals(1L, mergedCaptor.firstValue.oldestMessageId) + assertEquals(44L, mergedCaptor.firstValue.newestMessageId) + } + + @Test + fun `pullAndPersistMessages skips the block update when nothing was persisted`() = + runTest { + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(emptyList())) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(43)))) + wheneverBlocking { chatDao.upsertChatMessagesAndDeleteTemp(any(), any()) } + .thenThrow(SQLiteConstraintException()) + + val fieldMap = syncer.buildFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = 42 + ) + val outcome = syncer.pullAndPersistMessages(target(), fieldMap) + + assertFalse(outcome.persistedNewMessages) + assertNull(outcome.newestPersistedMessageId) + verifyBlocking(chatBlocksDao, never()) { upsertChatBlock(any()) } + } + + @Test + fun `pullAndPersistMessages treats not modified as no new messages`() = + runTest { + val rawResponse = okhttp3.Response.Builder() + .request(Request.Builder().url(CHAT_URL).build()) + .protocol(Protocol.HTTP_1_1) + .code(HTTP_NOT_MODIFIED) + .message("Not Modified") + .build() + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.error("".toResponseBody(), rawResponse)) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(emptyList())) + + val fieldMap = syncer.buildFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = 42 + ) + val outcome = syncer.pullAndPersistMessages(target(), fieldMap) + + assertFalse(outcome.persistedNewMessages) + verifyBlocking(chatDao, never()) { upsertChatMessagesAndDeleteTemp(any(), any()) } + } + + @Test + fun `cleanupExpiredMessages trims block boundaries and deletes empty blocks`() = + runTest { + val partiallyExpiredBlock = block(oldest = 1, newest = 10) + val fullyExpiredBlock = block(oldest = 20, newest = 30) + wheneverBlocking { chatDao.deleteExpiredMessages(eq(INTERNAL_CONVERSATION_ID), any()) } + .thenReturn(2) + wheneverBlocking { chatBlocksDao.getChatBlocksForConversation(INTERNAL_CONVERSATION_ID) } + .thenReturn(listOf(partiallyExpiredBlock, fullyExpiredBlock)) + wheneverBlocking { chatDao.getNewestMessageIdInRange(INTERNAL_CONVERSATION_ID, null, 1L, 10L) } + .thenReturn(8L) + wheneverBlocking { chatDao.getOldestMessageIdInRange(INTERNAL_CONVERSATION_ID, null, 1L, 10L) } + .thenReturn(2L) + wheneverBlocking { chatDao.getNewestMessageIdInRange(INTERNAL_CONVERSATION_ID, null, 20L, 30L) } + .thenReturn(null) + + syncer.cleanupExpiredMessages(INTERNAL_CONVERSATION_ID) + + val blockCaptor = argumentCaptor() + verifyBlocking(chatBlocksDao) { upsertChatBlock(blockCaptor.capture()) } + assertEquals(2L, blockCaptor.firstValue.oldestMessageId) + assertEquals(8L, blockCaptor.firstValue.newestMessageId) + + verify(chatBlocksDao).deleteChatBlocks(listOf(fullyExpiredBlock)) + } + + @Test + fun `cleanupExpiredMessages leaves blocks untouched when nothing expired`() = + runTest { + wheneverBlocking { chatDao.deleteExpiredMessages(eq(INTERNAL_CONVERSATION_ID), any()) } + .thenReturn(0) + + syncer.cleanupExpiredMessages(INTERNAL_CONVERSATION_ID) + + verifyNoInteractions(chatBlocksDao) + } + + private fun user(withKeepNotificationsCapability: Boolean = true): User { + val features = if (withKeepNotificationsCapability) { + listOf("chat-keep-notifications") + } else { + emptyList() + } + return User( + id = ACCOUNT_ID, + userId = "me", + username = "me", + baseUrl = "https://server.example.com", + capabilities = Capabilities().apply { + spreedCapability = SpreedCapability().apply { this.features = features } + } + ) + } + + private fun target(user: User = user()): ChatMessageSyncer.SyncTarget = + ChatMessageSyncer.SyncTarget( + user = user, + roomToken = ROOM_TOKEN, + threadId = null, + credentials = CREDENTIALS, + urlForChatting = CHAT_URL + ) + + private fun block(oldest: Long, newest: Long): ChatBlockEntity = + ChatBlockEntity( + internalConversationId = INTERNAL_CONVERSATION_ID, + accountId = ACCOUNT_ID, + token = ROOM_TOKEN, + threadId = null, + oldestMessageId = oldest, + newestMessageId = newest, + hasHistory = true + ) + + private fun message(id: Long): ChatMessageJson = + ChatMessageJson( + id = id, + token = ROOM_TOKEN, + actorType = "users", + actorId = "other", + actorDisplayName = "Other User", + timestamp = id, + message = "message $id", + messageType = "comment", + systemMessageType = ChatMessage.SystemMessageType.DUMMY + ) + + private fun overall(vararg messages: ChatMessageJson): ChatOverall = + ChatOverall(ocs = ChatOCS(meta = null, data = messages.toList())) + + companion object { + private const val ACCOUNT_ID = 1L + private const val ROOM_TOKEN = "room1" + private const val INTERNAL_CONVERSATION_ID = "$ACCOUNT_ID@$ROOM_TOKEN" + private const val CREDENTIALS = "credentials" + private const val CHAT_URL = "https://server.example.com/ocs/v2.php/apps/spreed/api/v1/chat/$ROOM_TOKEN" + private const val HTTP_NOT_MODIFIED = 304 + } +} diff --git a/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/RoomListMessagePrefetchIntegrationTest.kt b/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/RoomListMessagePrefetchIntegrationTest.kt new file mode 100644 index 00000000000..ee64de6ea60 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/conversationlist/data/network/RoomListMessagePrefetchIntegrationTest.kt @@ -0,0 +1,196 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Andy Scherzinger + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.conversationlist.data.network + +import android.app.Application +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import com.nextcloud.talk.chat.data.model.ChatMessage +import com.nextcloud.talk.chat.data.network.ChatMessageSyncer +import com.nextcloud.talk.chat.data.network.ChatNetworkDataSource +import com.nextcloud.talk.data.network.NetworkMonitor +import com.nextcloud.talk.data.source.local.TalkDatabase +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.data.user.model.UserEntity +import com.nextcloud.talk.models.json.capabilities.Capabilities +import com.nextcloud.talk.models.json.capabilities.SpreedCapability +import com.nextcloud.talk.models.json.chat.ChatMessageJson +import com.nextcloud.talk.models.json.chat.ChatOCS +import com.nextcloud.talk.models.json.chat.ChatOverall +import com.nextcloud.talk.models.json.conversations.Conversation +import com.nextcloud.talk.utils.ApiUtils +import io.reactivex.Observable +import io.reactivex.android.plugins.RxAndroidPlugins +import io.reactivex.schedulers.Schedulers +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import org.mockito.kotlin.wheneverBlocking +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import retrofit2.Response + +/** + * Integration test for the room list message prefetch: a room list sync with unread rooms must + * leave the chat messages of those rooms in the local database, covered by a contiguous chat + * block reaching the conversation's last message — so opening the chat needs no network request. + */ +@RunWith(RobolectricTestRunner::class) +@Config(application = Application::class, sdk = [33]) +class RoomListMessagePrefetchIntegrationTest { + + private lateinit var db: TalkDatabase + private lateinit var syncer: ChatMessageSyncer + private lateinit var repository: OfflineFirstConversationsRepository + + private val conversationsNetwork: ConversationsNetworkDataSource = mock() + private val chatNetwork: ChatNetworkDataSource = mock() + private val networkMonitor: NetworkMonitor = mock() + + @Before + fun setUp() { + RxAndroidPlugins.setInitMainThreadSchedulerHandler { Schedulers.trampoline() } + RxAndroidPlugins.setMainThreadSchedulerHandler { Schedulers.trampoline() } + + val context = ApplicationProvider.getApplicationContext() + db = Room.inMemoryDatabaseBuilder(context, TalkDatabase::class.java) + .allowMainThreadQueries() + .build() + db.usersDao().saveUser(UserEntity(id = ACCOUNT_ID, userId = "me", username = "me", baseUrl = BASE_URL)) + + whenever(networkMonitor.isOnline).thenReturn(MutableStateFlow(true)) + + syncer = ChatMessageSyncer(db.chatMessagesDao(), db.chatBlocksDao(), chatNetwork, networkMonitor) + repository = OfflineFirstConversationsRepository( + db.conversationsDao(), + conversationsNetwork, + chatNetwork, + networkMonitor, + syncer, + context + ) + } + + @After + fun tearDown() { + db.close() + RxAndroidPlugins.reset() + } + + @Test + fun `room list sync prefetches unread messages into contiguous chat blocks`() { + val rooms = listOf( + conversation(ROOM_A, unreadMessages = 2, lastMessageId = 12), + conversation(ROOM_B, unreadMessages = 1, lastMessageId = 7) + ) + whenever(conversationsNetwork.getRooms(any(), any(), any())).thenReturn(Observable.just(rooms)) + wheneverBlocking { chatNetwork.pullChatMessages(any(), eq(chatUrl(ROOM_A)), any()) } + .thenReturn(Response.success(overall(message(10, ROOM_A), message(11, ROOM_A), message(12, ROOM_A)))) + wheneverBlocking { chatNetwork.pullChatMessages(any(), eq(chatUrl(ROOM_B)), any()) } + .thenReturn(Response.success(overall(message(6, ROOM_B), message(7, ROOM_B)))) + + runBlocking { + repository.getRooms(user()).join() + + // the message catch-up runs as a fire-and-forget coroutine after the room list sync + awaitUntil { + newestBlockMessageId(ROOM_A) == 12L && newestBlockMessageId(ROOM_B) == 7L + } + + assertMessagesUpToLastMessage(ROOM_A, oldestFetched = 10L, lastMessageId = 12L) + assertMessagesUpToLastMessage(ROOM_B, oldestFetched = 6L, lastMessageId = 7L) + } + } + + private suspend fun assertMessagesUpToLastMessage(roomToken: String, oldestFetched: Long, lastMessageId: Long) { + val internalConversationId = "$ACCOUNT_ID@$roomToken" + + val newestCachedMessage = db.chatMessagesDao().getNewestMessageIdInRange( + internalConversationId = internalConversationId, + threadId = null, + oldestMessageId = 0, + newestMessageId = Long.MAX_VALUE + ) + assertEquals("newest cached message must reach lastMessage.id", lastMessageId, newestCachedMessage) + + val blocks = db.chatBlocksDao().getChatBlocksForConversation(internalConversationId) + assertEquals("exactly one contiguous chat block expected", 1, blocks.size) + assertEquals(oldestFetched, blocks[0].oldestMessageId) + assertEquals(lastMessageId, blocks[0].newestMessageId) + } + + private fun newestBlockMessageId(roomToken: String): Long = + db.chatBlocksDao().getNewestMessageIdFromChatBlocks("$ACCOUNT_ID@$roomToken", null) + + private suspend fun awaitUntil(timeoutMillis: Long = TIMEOUT_MILLIS, condition: () -> Boolean) { + val start = System.currentTimeMillis() + while (!condition()) { + if (System.currentTimeMillis() - start > timeoutMillis) { + throw AssertionError("Condition not met within $timeoutMillis ms") + } + delay(POLL_INTERVAL_MILLIS) + } + } + + private fun user(): User = + User( + id = ACCOUNT_ID, + userId = "me", + username = "me", + baseUrl = BASE_URL, + token = "app-password", + capabilities = Capabilities().apply { + spreedCapability = SpreedCapability().apply { features = listOf("chat-keep-notifications") } + } + ) + + private fun conversation(roomToken: String, unreadMessages: Int, lastMessageId: Long): Conversation = + Conversation( + token = roomToken, + lastActivity = lastMessageId, + unreadMessages = unreadMessages, + lastMessage = message(lastMessageId, roomToken) + ) + + private fun message(id: Long, roomToken: String): ChatMessageJson = + ChatMessageJson( + id = id, + token = roomToken, + actorType = "users", + actorId = "other", + actorDisplayName = "Other User", + timestamp = id, + message = "message $id", + messageType = "comment", + systemMessageType = ChatMessage.SystemMessageType.DUMMY + ) + + private fun overall(vararg messages: ChatMessageJson): ChatOverall = + ChatOverall(ocs = ChatOCS(meta = null, data = messages.toList())) + + private fun chatUrl(roomToken: String): String = ApiUtils.getUrlForChat(1, BASE_URL, roomToken) + + companion object { + private const val ACCOUNT_ID = 1L + private const val BASE_URL = "https://server.example.com" + private const val ROOM_A = "roomA" + private const val ROOM_B = "roomB" + private const val TIMEOUT_MILLIS = 10_000L + private const val POLL_INTERVAL_MILLIS = 50L + } +} From acd87f7c39b9cecac1771239a958c292e09acf6b Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 4 Aug 2026 20:19:14 +0200 Subject: [PATCH 13/27] feat: Add extra logging to spot pre-fetching in debug mode Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 36 ++++++++++++++----- .../data/network/ChatMessageSyncerTest.kt | 4 +++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 0a0ba78d86a..f506f39d441 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -80,7 +80,12 @@ class ChatMessageSyncer @Inject constructor( } } - data class SyncOutcome(val persistedNewMessages: Boolean, val newestPersistedMessageId: Long?) + data class SyncOutcome( + val persistedNewMessages: Boolean, + val newestPersistedMessageId: Long?, + val oldestPersistedMessageId: Long? = null, + val persistedMessageCount: Int = 0 + ) /** * Builds the query parameters for a chat pull request. setReadMarker stays 0 so a sync never @@ -231,7 +236,20 @@ class ChatMessageSyncer @Inject constructor( ) } - return pullAndPersistMessages(target, fieldMap) + val outcome = pullAndPersistMessages(target, fieldMap) + + if (outcome.persistedNewMessages) { + Log.d( + TAG, + "Background catch-up for room ${target.roomToken}: fetched ${outcome.persistedMessageCount} " + + "message(s), ids ${outcome.oldestPersistedMessageId}..${outcome.newestPersistedMessageId}" + + if (newestMessageIdFromDb > 0) " (delta from $newestMessageIdFromDb)" else " (initial fetch)" + ) + } else { + Log.d(TAG, "Background catch-up for room ${target.roomToken}: no new messages") + } + + return outcome } fun pullMessagesFlow(target: SyncTarget, fieldMap: HashMap): Flow = @@ -315,7 +333,7 @@ class ChatMessageSyncer @Inject constructor( } if (result.messages.isNotEmpty()) { - val newestPersistedId = updateMessagesData( + val persistedMessages = updateMessagesData( target, result.messages, blockContainingQueriedMessage, @@ -324,8 +342,10 @@ class ChatMessageSyncer @Inject constructor( events ) return SyncOutcome( - persistedNewMessages = newestPersistedId != null, - newestPersistedMessageId = newestPersistedId + persistedNewMessages = persistedMessages.isNotEmpty(), + newestPersistedMessageId = persistedMessages.maxOfOrNull { it.id }, + oldestPersistedMessageId = persistedMessages.minOfOrNull { it.id }, + persistedMessageCount = persistedMessages.size ) } else { Log.d(TAG, "No new messages to update") @@ -361,7 +381,7 @@ class ChatMessageSyncer @Inject constructor( lookIntoFuture: Boolean, hasHistory: Boolean, events: Events - ): Long? { + ): List { val chatMessageEntities = persistChatMessagesAndHandleSystemMessages( target, chatMessagesJson, @@ -374,7 +394,7 @@ class ChatMessageSyncer @Inject constructor( // persistChatMessagesAndHandleSystemMessages). Without persisted messages there must be // no chat block update either, otherwise a block would reference missing messages. Log.w(TAG, "No messages were persisted for ${target.internalConversationId}, skipping chat block update") - return null + return emptyList() } val oldestIdFromSync = chatMessageEntities.minByOrNull { it.id }!!.id @@ -411,7 +431,7 @@ class ChatMessageSyncer @Inject constructor( ) updateBlocks(target, newChatBlock) - return newestIdFromSync + return chatMessageEntities } suspend fun persistChatMessagesAndHandleSystemMessages( diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 720b4de680d..21c1b2e21f1 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -127,6 +127,8 @@ class ChatMessageSyncerTest { assertTrue(outcome.persistedNewMessages) assertEquals(44L, outcome.newestPersistedMessageId) + assertEquals(43L, outcome.oldestPersistedMessageId) + assertEquals(2, outcome.persistedMessageCount) val fieldMapCaptor = argumentCaptor>() verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) } @@ -154,6 +156,8 @@ class ChatMessageSyncerTest { assertTrue(outcome.persistedNewMessages) assertEquals(3L, outcome.newestPersistedMessageId) + assertEquals(1L, outcome.oldestPersistedMessageId) + assertEquals(3, outcome.persistedMessageCount) val fieldMapCaptor = argumentCaptor>() verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) } From bac2d79921a001f3fd5c4e2c67dfe168c26b7342 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 4 Aug 2026 20:48:02 +0200 Subject: [PATCH 14/27] refactor(chat): restructure catch-up guards to satisfy ReturnCount Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 131 ++++++++++-------- .../OfflineFirstConversationsRepository.kt | 37 ++--- .../nextcloud/talk/jobs/NotificationWorker.kt | 9 +- 3 files changed, 97 insertions(+), 80 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index f506f39d441..6b894ad07ba 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -196,21 +196,26 @@ class ChatMessageSyncer @Inject constructor( * dismiss the user's push notifications for the fetched messages, so the catch-up is skipped * entirely (same guard as on iOS). */ - suspend fun catchUpRoom(target: SyncTarget, limit: Int = DEFAULT_MESSAGES_LIMIT): SyncOutcome { - if (!networkMonitor.isOnline.value) { - Log.d(TAG, "Device is offline, skipping catch-up for ${target.internalConversationId}") - return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) - } + suspend fun catchUpRoom(target: SyncTarget, limit: Int = DEFAULT_MESSAGES_LIMIT): SyncOutcome = + when { + !networkMonitor.isOnline.value -> { + Log.d(TAG, "Device is offline, skipping catch-up for ${target.internalConversationId}") + NOTHING_SYNCED + } - if (!target.user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value)) { - Log.d( - TAG, - "Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, " + - "skipping catch-up for ${target.internalConversationId}" - ) - return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + !target.user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value) -> { + Log.d( + TAG, + "Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, " + + "skipping catch-up for ${target.internalConversationId}" + ) + NOTHING_SYNCED + } + + else -> fetchRoomCatchUp(target, limit) } + private suspend fun fetchRoomCatchUp(target: SyncTarget, limit: Int): SyncOutcome { val newestMessageIdFromDb = chatBlocksDao.getNewestMessageIdFromChatBlocks(target.internalConversationId, target.threadId) @@ -293,7 +298,6 @@ class ChatMessageSyncer @Inject constructor( * Pulls messages from the server as described by [fieldMap], persists them and updates the * chat blocks of [target]. */ - @Suppress("LongMethod") suspend fun pullAndPersistMessages( target: SyncTarget, fieldMap: HashMap, @@ -309,63 +313,23 @@ class ChatMessageSyncer @Inject constructor( val queriedMessageId = fieldMap["lastKnownMessageId"] val lookIntoFuture = fieldMap["lookIntoFuture"] == 1 - val result = pullMessagesFlow(target, fieldMap).first() - - when (result) { - is ChatPullResult.Success -> { - events.onLastCommonReadChanged(result.lastCommonRead) - - val hasHistory = getHasHistory(HTTP_CODE_OK, lookIntoFuture) - - Log.d( - TAG, - "internalConv=${target.internalConversationId} statusCode=$HTTP_CODE_OK " + - "lookIntoFuture=$lookIntoFuture hasHistory=$hasHistory " + - "queriedMessageId=$queriedMessageId" - ) - - val blockContainingQueriedMessage: ChatBlockEntity? = getBlockOfMessage(target, queriedMessageId) - - blockContainingQueriedMessage?.takeIf { !hasHistory }?.apply { - this.hasHistory = false - chatBlocksDao.upsertChatBlock(this) - Log.d(TAG, "End of chat reached, set hasHistory=false") - } - - if (result.messages.isNotEmpty()) { - val persistedMessages = updateMessagesData( - target, - result.messages, - blockContainingQueriedMessage, - lookIntoFuture, - hasHistory, - events - ) - return SyncOutcome( - persistedNewMessages = persistedMessages.isNotEmpty(), - newestPersistedMessageId = persistedMessages.maxOfOrNull { it.id }, - oldestPersistedMessageId = persistedMessages.minOfOrNull { it.id }, - persistedMessageCount = persistedMessages.size - ) - } else { - Log.d(TAG, "No new messages to update") - return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) - } - } + return when (val result = pullMessagesFlow(target, fieldMap).first()) { + is ChatPullResult.Success -> + handleSuccessfulPull(target, result, queriedMessageId, lookIntoFuture, events) is ChatPullResult.NotModified -> { Log.d(TAG, "Server returned NOT_MODIFIED, nothing to update") - return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + NOTHING_SYNCED } is ChatPullResult.PreconditionFailed -> { Log.d(TAG, "Server returned PRECONDITION_FAILED, nothing to update") - return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + NOTHING_SYNCED } is ChatPullResult.Error -> { Log.e(TAG, "Error pulling messages from server", result.throwable) - return SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + NOTHING_SYNCED } } } finally { @@ -373,6 +337,53 @@ class ChatMessageSyncer @Inject constructor( } } + private suspend fun handleSuccessfulPull( + target: SyncTarget, + result: ChatPullResult.Success, + queriedMessageId: Int?, + lookIntoFuture: Boolean, + events: Events + ): SyncOutcome { + events.onLastCommonReadChanged(result.lastCommonRead) + + val hasHistory = getHasHistory(HTTP_CODE_OK, lookIntoFuture) + + Log.d( + TAG, + "internalConv=${target.internalConversationId} statusCode=$HTTP_CODE_OK " + + "lookIntoFuture=$lookIntoFuture hasHistory=$hasHistory " + + "queriedMessageId=$queriedMessageId" + ) + + val blockContainingQueriedMessage: ChatBlockEntity? = getBlockOfMessage(target, queriedMessageId) + + blockContainingQueriedMessage?.takeIf { !hasHistory }?.apply { + this.hasHistory = false + chatBlocksDao.upsertChatBlock(this) + Log.d(TAG, "End of chat reached, set hasHistory=false") + } + + return if (result.messages.isNotEmpty()) { + val persistedMessages = updateMessagesData( + target, + result.messages, + blockContainingQueriedMessage, + lookIntoFuture, + hasHistory, + events + ) + SyncOutcome( + persistedNewMessages = persistedMessages.isNotEmpty(), + newestPersistedMessageId = persistedMessages.maxOfOrNull { it.id }, + oldestPersistedMessageId = persistedMessages.minOfOrNull { it.id }, + persistedMessageCount = persistedMessages.size + ) + } else { + Log.d(TAG, "No new messages to update") + NOTHING_SYNCED + } + } + @Suppress("LongParameterList") private suspend fun updateMessagesData( target: SyncTarget, @@ -673,6 +684,8 @@ class ChatMessageSyncer @Inject constructor( val NO_EVENTS: Events = object : Events {} + private val NOTHING_SYNCED = SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + private const val DEFAULT_MESSAGES_LIMIT = 100 private const val MILLIS_PER_SECOND = 1000L private const val HTTP_CODE_OK: Int = 200 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 115d7a64913..80440e73acb 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 @@ -214,22 +214,7 @@ class OfflineFirstConversationsRepository @Inject constructor( * requests, so a fresh install with many rooms cannot cause an unbounded request burst. */ private suspend fun catchUpRoomsWithNewMessages(user: User, rooms: List) { - if (rooms.isEmpty()) { - return - } - - if (!user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value)) { - Log.d(TAG, "Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, skipping message catch-up") - return - } - - if (isPowerSaveMode()) { - Log.d(TAG, "Battery saver is active, skipping message catch-up") - return - } - - if (isBackgroundDataRestricted()) { - Log.d(TAG, "Background data is restricted on a metered network, skipping message catch-up") + if (rooms.isEmpty() || !isCatchUpAllowed(user)) { return } @@ -263,6 +248,26 @@ class OfflineFirstConversationsRepository @Inject constructor( } } + private fun isCatchUpAllowed(user: User): Boolean = + when { + !user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value) -> { + Log.d(TAG, "Server lacks ${SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value}, skipping message catch-up") + false + } + + isPowerSaveMode() -> { + Log.d(TAG, "Battery saver is active, skipping message catch-up") + false + } + + isBackgroundDataRestricted() -> { + Log.d(TAG, "Background data is restricted on a metered network, skipping message catch-up") + false + } + + else -> true + } + private fun isPowerSaveMode(): Boolean { val powerManager = context.getSystemService(Context.POWER_SERVICE) as PowerManager return powerManager.isPowerSaveMode diff --git a/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt index 835ec532313..b9eacd2fd7c 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt @@ -229,11 +229,10 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor * check are handled inside [ChatMessageSyncer.catchUpRoom]. */ private fun catchUpPushedRoom() { - val roomToken = pushMessage.id ?: return - val syncer = chatMessageSyncer ?: return - - if (isPowerSaveMode()) { - logger.d(TAG, "Battery saver is active, skipping message catch-up for pushed room") + val roomToken = pushMessage.id + val syncer = chatMessageSyncer + if (roomToken == null || syncer == null || isPowerSaveMode()) { + logger.d(TAG, "Skipping message catch-up for pushed room (missing data or battery saver active)") return } From 9a519fb72f2a8bc47cfd70525a7aa45402775393 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 6 Aug 2026 19:46:36 +0200 Subject: [PATCH 15/27] feat(chat): coalesce bursts of background message catch-ups Run at most one catch-up per room at a time: requests arriving while one runs only mark a rerun that the running catch-up executes after finishing, consecutive fetches are paced by a five second cooldown and a burst performs at most three fetches. A flood of push notifications for an active room now causes one or two delta fetches instead of one per push. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 74 ++++++++++++++++++- .../data/network/ChatMessageSyncerTest.kt | 46 ++++++++++++ 2 files changed, 116 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 6b894ad07ba..5083e32e2a6 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -8,6 +8,7 @@ package com.nextcloud.talk.chat.data.network import android.database.sqlite.SQLiteConstraintException +import android.os.SystemClock import android.util.Log import com.nextcloud.talk.chat.data.model.ChatMessage import com.nextcloud.talk.chat.domain.ChatPullResult @@ -21,20 +22,26 @@ import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.models.json.chat.ChatMessageJson import com.nextcloud.talk.utils.SpreedFeatures import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.sync.Mutex import retrofit2.HttpException +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicBoolean import javax.inject.Inject /** * The chat message fetch-and-persist core, shared between the chat screen * ([OfflineFirstChatRepository]) and background sync callers. * - * The syncer holds no per-conversation state: every operation takes a [SyncTarget] describing the - * account, room and thread to sync, so it can be used for any room at any time — no open chat - * required. UI-bound side effects are reported through the optional [Events] listener. + * Every operation takes a [SyncTarget] describing the account, room and thread to sync, so the + * syncer can be used for any room at any time — no open chat required. UI-bound side effects are + * reported through the optional [Events] listener. The only state kept between calls is the + * 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") class ChatMessageSyncer @Inject constructor( @@ -195,6 +202,10 @@ class ChatMessageSyncer @Inject constructor( * Requires the chat-keep-notifications capability: without it, a background fetch would * dismiss the user's push notifications for the fetched messages, so the catch-up is skipped * entirely (same guard as on iOS). + * + * Bursts of catch-up requests for the same room (e.g. one push notification per message of an + * active group chat) are coalesced: while a catch-up runs, further requests only mark a rerun + * and return, and consecutive fetches are paced by [CATCH_UP_COOLDOWN_MILLIS]. */ suspend fun catchUpRoom(target: SyncTarget, limit: Int = DEFAULT_MESSAGES_LIMIT): SyncOutcome = when { @@ -212,8 +223,61 @@ class ChatMessageSyncer @Inject constructor( NOTHING_SYNCED } - else -> fetchRoomCatchUp(target, limit) + else -> coalescedRoomCatchUp(target, limit) + } + + /** + * Runs at most one catch-up per room at a time. A request arriving while one is running only + * marks a rerun: the running catch-up re-fetches once more after finishing, so messages that + * arrived in between are still picked up without a parallel request. Consecutive fetches are + * paced by [CATCH_UP_COOLDOWN_MILLIS] and a single burst performs at most + * [MAX_CATCH_UP_RUNS_PER_BURST] fetches — later messages are covered by their own push or the + * next room list sync. + */ + private suspend fun coalescedRoomCatchUp(target: SyncTarget, limit: Int): SyncOutcome { + val stateKey = "${target.internalConversationId}#${target.threadId}" + val state = catchUpStates.getOrPut(stateKey) { RoomCatchUpState() } + + if (!state.mutex.tryLock()) { + state.rerunRequested.set(true) + Log.d(TAG, "Catch-up already running for $stateKey, coalescing into it") + return NOTHING_SYNCED + } + + try { + var outcome: SyncOutcome + var runs = 0 + do { + awaitCatchUpCooldown(state, stateKey) + state.rerunRequested.set(false) + outcome = fetchRoomCatchUp(target, limit) + state.lastCompletedAtMillis = SystemClock.elapsedRealtime() + runs++ + } while (state.rerunRequested.get() && runs < MAX_CATCH_UP_RUNS_PER_BURST) + return outcome + } finally { + state.mutex.unlock() } + } + + private suspend fun awaitCatchUpCooldown(state: RoomCatchUpState, stateKey: String) { + val elapsedSinceLastCatchUp = SystemClock.elapsedRealtime() - state.lastCompletedAtMillis + val remainingCooldown = CATCH_UP_COOLDOWN_MILLIS - elapsedSinceLastCatchUp + if (state.lastCompletedAtMillis > 0 && remainingCooldown > 0) { + Log.d(TAG, "Catch-up cooldown for $stateKey, delaying fetch by $remainingCooldown ms") + delay(remainingCooldown) + } + } + + private class RoomCatchUpState { + val mutex = Mutex() + val rerunRequested = AtomicBoolean(false) + + @Volatile + var lastCompletedAtMillis = 0L + } + + private val catchUpStates = ConcurrentHashMap() private suspend fun fetchRoomCatchUp(target: SyncTarget, limit: Int): SyncOutcome { val newestMessageIdFromDb = @@ -688,6 +752,8 @@ class ChatMessageSyncer @Inject constructor( private const val DEFAULT_MESSAGES_LIMIT = 100 private const val MILLIS_PER_SECOND = 1000L + private const val CATCH_UP_COOLDOWN_MILLIS = 5_000L + private const val MAX_CATCH_UP_RUNS_PER_BURST = 3 private const val HTTP_CODE_OK: Int = 200 private const val HTTP_CODE_NOT_MODIFIED = 304 private const val HTTP_CODE_PRECONDITION_FAILED = 412 diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 21c1b2e21f1..33bcd5c7780 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -19,8 +19,10 @@ import com.nextcloud.talk.models.json.capabilities.SpreedCapability import com.nextcloud.talk.models.json.chat.ChatMessageJson import com.nextcloud.talk.models.json.chat.ChatOCS import com.nextcloud.talk.models.json.chat.ChatOverall +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest import okhttp3.Protocol import okhttp3.Request @@ -33,9 +35,11 @@ import org.junit.Before import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyBlocking import org.mockito.kotlin.verifyNoInteractions @@ -172,6 +176,48 @@ class ChatMessageSyncerTest { assertEquals(3L, blockCaptor.firstValue.newestMessageId) } + @Test + fun `catchUpRoom coalesces a burst of calls for the same room into two fetches`() = + runTest { + val existingBlock = block(oldest = 10, newest = 42) + whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null)) + .thenReturn(42L) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(listOf(existingBlock))) + whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) + .thenReturn(flowOf(listOf(existingBlock))) + + val firstFetchStarted = CompletableDeferred() + val firstFetchReleased = CompletableDeferred() + var pullCount = 0 + wheneverBlocking { network.pullChatMessages(any(), any(), any()) }.doSuspendableAnswer { + pullCount++ + if (pullCount == 1) { + firstFetchStarted.complete(Unit) + firstFetchReleased.await() + } + Response.success(overall(message(43), message(44))) + } + + val firstCall = launch { syncer.catchUpRoom(target()) } + firstFetchStarted.await() + assertEquals("first fetch must be in flight", 1, pullCount) + + // a second and third call while the first fetch is in flight must not fetch in + // parallel — they only mark a rerun for the running catch-up + val secondOutcome = syncer.catchUpRoom(target()) + val thirdOutcome = syncer.catchUpRoom(target()) + assertFalse(secondOutcome.persistedNewMessages) + assertFalse(thirdOutcome.persistedNewMessages) + assertEquals(1, pullCount) + + firstFetchReleased.complete(Unit) + firstCall.join() + + // the running catch-up re-fetched exactly once for the whole burst + verifyBlocking(network, times(2)) { pullChatMessages(any(), any(), any()) } + } + @Test fun `pullAndPersistMessages merges connected chat blocks`() = runTest { From 733a8804c7fb279c93d1623c095fe99064b40c26 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 9 Aug 2026 12:22:32 +0200 Subject: [PATCH 16/27] fix(chat): track insurance anchor from http syncs only Reintroduce a sync-only anchor for the insurance request, now held per conversation in the singleton ChatMessageSyncer so it survives reopening a chat. It is updated exclusively from http pull results and seeded from the conversation's lastMessage on skip, never from signaling messages, so the insurance request keeps detecting messages that arrived between the last sync and signaling delivery. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 46 ++++++++++++++- .../network/OfflineFirstChatRepository.kt | 22 +++++-- .../data/network/ChatMessageSyncerTest.kt | 57 +++++++++++++++++++ 3 files changed, 118 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 5083e32e2a6..a3530b89015 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -235,7 +235,7 @@ class ChatMessageSyncer @Inject constructor( * next room list sync. */ private suspend fun coalescedRoomCatchUp(target: SyncTarget, limit: Int): SyncOutcome { - val stateKey = "${target.internalConversationId}#${target.threadId}" + val stateKey = syncStateKey(target.internalConversationId, target.threadId) val state = catchUpStates.getOrPut(stateKey) { RoomCatchUpState() } if (!state.mutex.tryLock()) { @@ -279,6 +279,39 @@ class ChatMessageSyncer @Inject constructor( private val catchUpStates = ConcurrentHashMap() + /** + * Newest message id per conversation/thread that is known from HTTP syncs — deliberately + * EXCLUDING messages delivered via signaling. Signaling messages extend the latest chat block + * optimistically (assuming they are contiguous on top of it); the insurance request exists to + * verify that assumption and must therefore anchor on the last HTTP-synced message, otherwise + * messages that arrived between the last sync and the signaling delivery would never be + * fetched. Living in this singleton, the anchor survives reopening a chat. + */ + private val lastHttpSyncedMessageIds = ConcurrentHashMap() + + /** + * The newest message id of the conversation/thread confirmed via HTTP sync, or null when no + * sync happened yet (and [seedHttpSyncedMessageId] was never called) since app start. + */ + fun lastHttpSyncedMessageId(internalConversationId: String, threadId: Long?): Long? = + lastHttpSyncedMessageIds[syncStateKey(internalConversationId, threadId)] + + /** + * Seeds the HTTP-synced anchor without a fetch. Only call with ids whose coverage is proven by + * HTTP-derived data — e.g. the conversation's lastMessage from the room list sync when the + * local chat block already reaches it. The anchor only ever moves forward. + */ + fun seedHttpSyncedMessageId(internalConversationId: String, threadId: Long?, messageId: Long) { + lastHttpSyncedMessageIds.merge(syncStateKey(internalConversationId, threadId), messageId, ::maxOf) + } + + private fun recordHttpSyncedMessageId(target: SyncTarget, messageId: Long) { + lastHttpSyncedMessageIds.merge(syncStateKey(target.internalConversationId, target.threadId), messageId, ::maxOf) + } + + private fun syncStateKey(internalConversationId: String, threadId: Long?): String = + "$internalConversationId#$threadId" + private suspend fun fetchRoomCatchUp(target: SyncTarget, limit: Int): SyncOutcome { val newestMessageIdFromDb = chatBlocksDao.getNewestMessageIdFromChatBlocks(target.internalConversationId, target.threadId) @@ -383,6 +416,11 @@ class ChatMessageSyncer @Inject constructor( is ChatPullResult.NotModified -> { Log.d(TAG, "Server returned NOT_MODIFIED, nothing to update") + if (lookIntoFuture && queriedMessageId != null) { + // the server confirmed there is nothing newer than the queried message, so + // the queried message is a valid HTTP-synced anchor + recordHttpSyncedMessageId(target, queriedMessageId.toLong()) + } NOTHING_SYNCED } @@ -436,6 +474,7 @@ class ChatMessageSyncer @Inject constructor( hasHistory, events ) + persistedMessages.maxOfOrNull { it.id }?.let { recordHttpSyncedMessageId(target, it) } SyncOutcome( persistedNewMessages = persistedMessages.isNotEmpty(), newestPersistedMessageId = persistedMessages.maxOfOrNull { it.id }, @@ -444,6 +483,11 @@ class ChatMessageSyncer @Inject constructor( ) } else { Log.d(TAG, "No new messages to update") + if (lookIntoFuture && queriedMessageId != null) { + // an empty response to a lookIntoFuture request confirms the queried message as + // the newest one, so it is a valid HTTP-synced anchor + recordHttpSyncedMessageId(target, queriedMessageId.toLong()) + } NOTHING_SYNCED } } diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index d63f317bb7e..f2e0347bec1 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -205,6 +205,14 @@ class OfflineFirstChatRepository @Inject constructor( "Initial online request is skipped because offline messages are up to date" + " until the conversation's last message" ) + + // No HTTP fetch happens in this branch, so seed the insurance anchor explicitly: + // lastMessage came from the room list sync (HTTP) and the local chat block reaches + // it, so it is a valid HTTP-synced anchor. Without the seed, the first insurance + // request of this session would query with lastKnownMessageId=0. + conversationModel.lastMessage?.id?.let { + syncer.seedHttpSyncedMessageId(internalConversationId, threadId, it) + } } weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage -> { @@ -333,24 +341,26 @@ class OfflineFirstChatRepository @Inject constructor( } /** - * Fetches messages newer than the newest message covered by the chat blocks. + * Fetches messages newer than the newest message known from HTTP syncs. * - * The anchor is read from the database instead of in-memory state: messages and chat blocks - * are persisted together, so the blocks are always at least as fresh — and unlike a field in - * this (unscoped, per-chat-open) repository they survive reopening the chat. + * The anchor deliberately EXCLUDES messages that were delivered via signaling: those extend + * the latest chat block optimistically (assuming they are contiguous on top of it), and this + * request is the insurance that verifies the assumption — anchoring on the chat block's + * newest message would skip exactly the range it has to check. The anchor lives in the + * singleton [ChatMessageSyncer], so it survives reopening the chat. * * @return `true` if at least one new message was received and persisted. */ override suspend fun fetchNewMessages(): Boolean { cleanupExpiredMessages() - val newestMessageIdFromDb = chatBlocksDao.getNewestMessageIdFromChatBlocks(internalConversationId, threadId) + val lastHttpSyncedMessageId = syncer.lastHttpSyncedMessageId(internalConversationId, threadId) ?: 0L val fieldMap = getFieldMap( lookIntoFuture = true, timeout = 0, includeLastKnown = false, - lastKnown = newestMessageIdFromDb.toInt(), + lastKnown = lastHttpSyncedMessageId.toInt(), limit = 200 ) val networkParams = Bundle() diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 33bcd5c7780..f89baaf6f53 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -292,6 +292,63 @@ class ChatMessageSyncerTest { verifyBlocking(chatDao, never()) { upsertChatMessagesAndDeleteTemp(any(), any()) } } + @Test + fun `http pulls record the insurance anchor but signaling persists do not`() = + runTest { + val existingBlock = block(oldest = 10, newest = 42) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(listOf(existingBlock))) + whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) + .thenReturn(flowOf(listOf(existingBlock))) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(43), message(44)))) + + val fieldMap = syncer.buildFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = 42 + ) + syncer.pullAndPersistMessages(target(), fieldMap) + + assertEquals(44L, syncer.lastHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null)) + + // a message delivered via signaling must NOT move the anchor: the insurance request + // has to verify the signaling assumption and needs the last HTTP-synced id for that + syncer.persistChatMessagesAndHandleSystemMessages(target(), listOf(message(50))) + + assertEquals(44L, syncer.lastHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null)) + } + + @Test + fun `an empty lookIntoFuture response confirms the queried anchor`() = + runTest { + whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) + .thenReturn(flowOf(emptyList())) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall())) + + val fieldMap = syncer.buildFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = 42 + ) + syncer.pullAndPersistMessages(target(), fieldMap) + + assertEquals(42L, syncer.lastHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null)) + } + + @Test + fun `seeded insurance anchor only moves forward`() { + assertNull(syncer.lastHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null)) + + syncer.seedHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null, 44L) + syncer.seedHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null, 10L) + + assertEquals(44L, syncer.lastHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null)) + } + @Test fun `cleanupExpiredMessages trims block boundaries and deletes empty blocks`() = runTest { From 22686cc11eda3f7ce5393c97680b13c662b9fe08 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 9 Aug 2026 14:20:20 +0200 Subject: [PATCH 17/27] fix(chat): loop delta fetches until the backlog is closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single delta fetch is capped by the request limit and may only narrow a large backlog. Repeat the fetch until the server returns fewer messages than the limit, bounded by a maximum round count, and fall back to fetching the newest messages with includeLastKnown when the bound is hit — so chat blocks never claim ranges that were not fetched and the chat relay path cannot create permanent gaps. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 118 +++++++++++++++--- .../network/OfflineFirstChatRepository.kt | 50 ++++---- .../data/network/ChatMessageSyncerTest.kt | 61 +++++++++ 3 files changed, 185 insertions(+), 44 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index a3530b89015..f74c860ec47 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -316,30 +316,28 @@ class ChatMessageSyncer @Inject constructor( val newestMessageIdFromDb = chatBlocksDao.getNewestMessageIdFromChatBlocks(target.internalConversationId, target.threadId) - val fieldMap = if (newestMessageIdFromDb > 0) { - buildFieldMap( - lookIntoFuture = true, - timeout = 0, - includeLastKnown = false, - lastKnown = newestMessageIdFromDb.toInt(), + val outcome = if (newestMessageIdFromDb > 0) { + closeBacklog( + target = target, + fromMessageId = newestMessageIdFromDb, limit = limit, - threadId = target.threadId, markNotificationsAsRead = false ) } else { - buildFieldMap( - lookIntoFuture = false, - timeout = 0, - includeLastKnown = true, - lastKnown = null, - limit = limit, - threadId = target.threadId, - markNotificationsAsRead = false + pullAndPersistMessages( + target, + buildFieldMap( + lookIntoFuture = false, + timeout = 0, + includeLastKnown = true, + lastKnown = null, + limit = limit, + threadId = target.threadId, + markNotificationsAsRead = false + ) ) } - val outcome = pullAndPersistMessages(target, fieldMap) - if (outcome.persistedNewMessages) { Log.d( TAG, @@ -354,6 +352,91 @@ class ChatMessageSyncer @Inject constructor( return outcome } + /** + * Fetches the messages newer than [fromMessageId] until the backlog is fully closed. + * + * A single fetch is capped by [limit], so one request only narrows a backlog larger than + * that — and on chat-relay servers the remaining gap would become permanent as soon as a + * signaling message extends the latest chat block over it. Fetches are therefore repeated + * until the server returns fewer messages than the limit. If [MAX_BACKLOG_ROUNDS] full pages + * were fetched and the backlog is still not closed, the newest messages are fetched with + * includeLastKnown instead: that creates a separate top chat block, so the remaining gap + * stays visible in the block structure (closable by scrolling up) rather than a block + * claiming ranges that were never fetched. + */ + @Suppress("LongParameterList") + suspend fun closeBacklog( + target: SyncTarget, + fromMessageId: Long, + limit: Int = DEFAULT_MESSAGES_LIMIT, + lastCommonRead: Int? = null, + markNotificationsAsRead: Boolean = true, + events: Events = NO_EVENTS + ): SyncOutcome { + var anchor = fromMessageId + var totalCount = 0 + var oldestPersisted: Long? = null + var newestPersisted: Long? = null + + repeat(MAX_BACKLOG_ROUNDS) { + val fieldMap = buildFieldMap( + lookIntoFuture = true, + timeout = 0, + includeLastKnown = false, + lastKnown = anchor.toInt(), + limit = limit, + threadId = target.threadId, + lastCommonRead = lastCommonRead, + markNotificationsAsRead = markNotificationsAsRead + ) + val roundOutcome = pullAndPersistMessages(target, fieldMap, events) + + if (roundOutcome.persistedNewMessages) { + totalCount += roundOutcome.persistedMessageCount + oldestPersisted = oldestPersisted ?: roundOutcome.oldestPersistedMessageId + newestPersisted = roundOutcome.newestPersistedMessageId ?: newestPersisted + } + + val caughtUp = !roundOutcome.persistedNewMessages || roundOutcome.persistedMessageCount < limit + val nextAnchor = roundOutcome.newestPersistedMessageId + if (caughtUp || nextAnchor == null) { + return SyncOutcome( + persistedNewMessages = totalCount > 0, + newestPersistedMessageId = newestPersisted, + oldestPersistedMessageId = oldestPersisted, + persistedMessageCount = totalCount + ) + } + anchor = nextAnchor + } + + Log.w( + TAG, + "Backlog above $fromMessageId in ${target.internalConversationId} still not closed after " + + "$MAX_BACKLOG_ROUNDS rounds, fetching the newest messages instead" + ) + val fallbackOutcome = pullAndPersistMessages( + target, + buildFieldMap( + lookIntoFuture = false, + timeout = 0, + includeLastKnown = true, + lastKnown = null, + limit = limit, + threadId = target.threadId, + lastCommonRead = lastCommonRead, + markNotificationsAsRead = markNotificationsAsRead + ), + events + ) + return SyncOutcome( + persistedNewMessages = totalCount > 0 || fallbackOutcome.persistedNewMessages, + newestPersistedMessageId = fallbackOutcome.newestPersistedMessageId ?: newestPersisted, + oldestPersistedMessageId = oldestPersisted ?: fallbackOutcome.oldestPersistedMessageId, + persistedMessageCount = totalCount + fallbackOutcome.persistedMessageCount + ) + } + fun pullMessagesFlow(target: SyncTarget, fieldMap: HashMap): Flow = flow { var attempts = 1 @@ -798,6 +881,7 @@ class ChatMessageSyncer @Inject constructor( private const val MILLIS_PER_SECOND = 1000L private const val CATCH_UP_COOLDOWN_MILLIS = 5_000L private const val MAX_CATCH_UP_RUNS_PER_BURST = 3 + private const val MAX_BACKLOG_ROUNDS = 5 private const val HTTP_CODE_OK: Int = 200 private const val HTTP_CODE_NOT_MODIFIED = 304 private const val HTTP_CODE_PRECONDITION_FAILED = 412 diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index f2e0347bec1..dfb35b608c3 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -216,25 +216,21 @@ class OfflineFirstChatRepository @Inject constructor( } weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage -> { - // Close the backlog since the newest offline message with a delta fetch. This is - // required on chat-relay servers (the relay cannot deliver messages that arrived - // while the app was closed) and is equally cheap on long-polling servers, where it - // just front-loads what the first poll request would have fetched. This way the + // Close the backlog since the newest offline message. This is required on + // chat-relay servers (the relay cannot deliver messages that arrived while the + // app was closed) and is equally cheap on long-polling servers, where it just + // front-loads what the first poll request would have fetched. This way the // initial load never has to know the live-update mode, i.e. it must not wait for - // the websocket. - Log.d(TAG, "A delta request from the newest offline message is made to close the backlog") - - val fieldMap = getFieldMap( - lookIntoFuture = true, - timeout = 0, - includeLastKnown = false, - lastKnown = newestMessageIdFromDb.toInt() + // the websocket. closeBacklog loops until the backlog is fully closed — a single + // capped fetch could leave a permanent gap on chat-relay servers. + Log.d(TAG, "Closing the backlog from the newest offline message for initial loading") + + syncer.closeBacklog( + target = syncTarget, + fromMessageId = newestMessageIdFromDb, + lastCommonRead = newXChatLastCommonRead, + events = syncEvents ) - withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) - withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token) - - Log.d(TAG, "Starting delta request for initial loading") - getAndPersistMessages(withNetworkParams) } else -> { @@ -356,17 +352,17 @@ class OfflineFirstChatRepository @Inject constructor( val lastHttpSyncedMessageId = syncer.lastHttpSyncedMessageId(internalConversationId, threadId) ?: 0L - val fieldMap = getFieldMap( - lookIntoFuture = true, - timeout = 0, - includeLastKnown = false, - lastKnown = lastHttpSyncedMessageId.toInt(), - limit = 200 + // closeBacklog loops until the backlog is fully closed, so a backlog larger than the + // request limit is caught up within one insurance cycle instead of narrowing it by one + // page every cycle. + val outcome = syncer.closeBacklog( + target = syncTarget, + fromMessageId = lastHttpSyncedMessageId, + limit = 200, + lastCommonRead = newXChatLastCommonRead, + events = syncEvents ) - val networkParams = Bundle() - networkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) - - return getAndPersistMessages(networkParams) + return outcome.persistedNewMessages } override suspend fun loadMoreMessages( diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index f89baaf6f53..53247ab87fe 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -292,6 +292,67 @@ class ChatMessageSyncerTest { verifyBlocking(chatDao, never()) { upsertChatMessagesAndDeleteTemp(any(), any()) } } + @Test + fun `closeBacklog loops until the server returns fewer messages than the limit`() = + runTest { + val existingBlock = block(oldest = 10, newest = 42) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(eq(INTERNAL_CONVERSATION_ID), eq(null), any())) + .thenReturn(flowOf(listOf(existingBlock))) + whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) + .thenReturn(flowOf(listOf(existingBlock))) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn( + Response.success(overall(message(43), message(44))), + Response.success(overall(message(45))) + ) + + val outcome = syncer.closeBacklog(target(), fromMessageId = 42, limit = 2) + + assertTrue(outcome.persistedNewMessages) + assertEquals(3, outcome.persistedMessageCount) + assertEquals(43L, outcome.oldestPersistedMessageId) + assertEquals(45L, outcome.newestPersistedMessageId) + + val fieldMapCaptor = argumentCaptor>() + verifyBlocking(network, times(2)) { pullChatMessages(any(), any(), fieldMapCaptor.capture()) } + assertEquals(42, fieldMapCaptor.firstValue["lastKnownMessageId"]) + assertEquals(44, fieldMapCaptor.secondValue["lastKnownMessageId"]) + } + + @Test + fun `closeBacklog falls back to the newest messages when the backlog persists`() = + runTest { + val existingBlock = block(oldest = 10, newest = 42) + whenever(chatBlocksDao.getChatBlocksContainingMessageId(eq(INTERNAL_CONVERSATION_ID), eq(null), any())) + .thenReturn(flowOf(listOf(existingBlock))) + whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) + .thenReturn(flowOf(listOf(existingBlock))) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn( + Response.success(overall(message(43))), + Response.success(overall(message(44))), + Response.success(overall(message(45))), + Response.success(overall(message(46))), + Response.success(overall(message(47))), + Response.success(overall(message(100))) + ) + + val outcome = syncer.closeBacklog(target(), fromMessageId = 42, limit = 1) + + assertTrue(outcome.persistedNewMessages) + assertEquals(6, outcome.persistedMessageCount) + assertEquals(43L, outcome.oldestPersistedMessageId) + assertEquals(100L, outcome.newestPersistedMessageId) + + val fieldMapCaptor = argumentCaptor>() + verifyBlocking(network, times(6)) { pullChatMessages(any(), any(), fieldMapCaptor.capture()) } + // the last request is the fallback: newest messages with includeLastKnown, no anchor + val fallbackFieldMap = fieldMapCaptor.allValues.last() + assertEquals(0, fallbackFieldMap["lookIntoFuture"]) + assertEquals(1, fallbackFieldMap["includeLastKnown"]) + assertFalse(fallbackFieldMap.containsKey("lastKnownMessageId")) + } + @Test fun `http pulls record the insurance anchor but signaling persists do not`() = runTest { From 5bd38f071798717ea7d37b7ed091fe1d48ab567e Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 9 Aug 2026 14:33:16 +0200 Subject: [PATCH 18/27] fix(notifications): catch up the pushed thread, not only the room Trigger the push catch-up from the notification data callback and pass the thread id parsed from the notification's objectId, so messages of a pushed thread land in the thread's chat block. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../nextcloud/talk/jobs/NotificationWorker.kt | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt index b9eacd2fd7c..4838b5c1be4 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt @@ -216,23 +216,21 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor private fun handleNonCallPushMessage() { val mainActivityIntent = createMainActivityIntent() getNcDataAndShowNotification(mainActivityIntent) - if (pushMessage.type == TYPE_CHAT) { - catchUpPushedRoom() - } } /** * Prefetches the pushed room's messages into the local database so they are instantly visible - * when the chat is opened from the notification (or later). Best effort only: failures are - * logged and never delay or suppress the notification, which is displayed independently. + * when the chat is opened from the notification (or later). For messages in a thread, + * [threadId] targets the thread so its chat block is extended. Best effort only: failures are + * logged and never delay or suppress the notification, which is displayed beforehand. * Skipped in battery saver mode; the chat-keep-notifications capability gate and the offline * check are handled inside [ChatMessageSyncer.catchUpRoom]. */ - private fun catchUpPushedRoom() { + private fun catchUpPushedRoom(threadId: Long?) { val roomToken = pushMessage.id val syncer = chatMessageSyncer - if (roomToken == null || syncer == null || isPowerSaveMode()) { - logger.d(TAG, "Skipping message catch-up for pushed room (missing data or battery saver active)") + if (pushMessage.type != TYPE_CHAT || roomToken == null || syncer == null || isPowerSaveMode()) { + logger.d(TAG, "Skipping message catch-up for pushed room (not a chat push or battery saver active)") return } @@ -243,7 +241,7 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor val target = ChatMessageSyncer.SyncTarget( user = currentUser, roomToken = roomToken, - threadId = null, + threadId = threadId, credentials = credentials, urlForChatting = ApiUtils.getUrlForChat(CHAT_API_VERSION, currentUser.baseUrl!!, roomToken) ) @@ -536,6 +534,7 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor threadId?.let { intent.putExtra(KEY_THREAD_ID, it) } showNotification(intent, ncNotification) + catchUpPushedRoom(threadId) } } @@ -550,6 +549,10 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor setContentsFromPushNotificationSubject() showNotification(intent, null) + // without the server notification the thread id is unknown — still catch up + // the room itself so the pushed message is cached for the main chat + catchUpPushedRoom(threadId = null) + Log.e(TAG, "Failed to get NC notification. Using decrypted data from push notification itself", e) if (BuildConfig.DEBUG) { Handler(Looper.getMainLooper()).post { From a0f0d9c8711c24fe437a5d888662fc8cbe72faa8 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 9 Aug 2026 14:36:23 +0200 Subject: [PATCH 19/27] chore: remove unrelated code change ...brought in via a second dev work strem not related to chat message fetching, hence removing Signed-off-by: Andy Scherzinger --- .../talk/chat/ui/VoiceRecordingLockFab.kt | 87 ------------------- 1 file changed, 87 deletions(-) delete mode 100644 app/src/main/java/com/nextcloud/talk/chat/ui/VoiceRecordingLockFab.kt diff --git a/app/src/main/java/com/nextcloud/talk/chat/ui/VoiceRecordingLockFab.kt b/app/src/main/java/com/nextcloud/talk/chat/ui/VoiceRecordingLockFab.kt deleted file mode 100644 index ba73dcff8a2..00000000000 --- a/app/src/main/java/com/nextcloud/talk/chat/ui/VoiceRecordingLockFab.kt +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Nextcloud Talk - Android Client - * - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: GPL-3.0-or-later - */ - -package com.nextcloud.talk.chat.ui - -import android.content.res.Configuration -import androidx.compose.animation.AnimatedVisibility -import androidx.compose.animation.fadeIn -import androidx.compose.animation.fadeOut -import androidx.compose.animation.scaleIn -import androidx.compose.animation.scaleOut -import androidx.compose.foundation.isSystemInDarkTheme -import androidx.compose.material3.FloatingActionButton -import androidx.compose.material3.Icon -import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.Surface -import androidx.compose.material3.darkColorScheme -import androidx.compose.material3.lightColorScheme -import androidx.compose.runtime.Composable -import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.graphicsLayer -import androidx.compose.ui.res.painterResource -import androidx.compose.ui.res.stringResource -import androidx.compose.ui.tooling.preview.Preview -import com.nextcloud.talk.R - -@Composable -fun VoiceRecordingLockFab(visible: Boolean, offsetY: Float, modifier: Modifier = Modifier) { - AnimatedVisibility( - visible = visible, - modifier = modifier, - enter = scaleIn() + fadeIn(), - exit = scaleOut() + fadeOut() - ) { - FloatingActionButton( - onClick = {}, - modifier = Modifier.graphicsLayer { translationY = offsetY }, - containerColor = MaterialTheme.colorScheme.primaryContainer, - contentColor = MaterialTheme.colorScheme.onPrimaryContainer - ) { - Icon( - painter = painterResource(R.drawable.ic_lock_open_grey600_24dp), - contentDescription = stringResource(R.string.continuous_voice_message_recording) - ) - } - } -} - -private const val PREVIEW_DRAG_OFFSET_PX = -80f - -@Preview(name = "Visible · default position · Light") -@Preview(name = "Visible · default position · Dark", uiMode = Configuration.UI_MODE_NIGHT_YES) -@Composable -private fun VisibleDefaultPreview() { - val colorScheme = if (isSystemInDarkTheme()) darkColorScheme() else lightColorScheme() - MaterialTheme(colorScheme = colorScheme) { - Surface { - VoiceRecordingLockFab(visible = true, offsetY = 0f) - } - } -} - -@Preview(name = "Visible · mid-drag · Light") -@Preview(name = "Visible · mid-drag · Dark", uiMode = Configuration.UI_MODE_NIGHT_YES) -@Composable -private fun VisibleDraggedPreview() { - val colorScheme = if (isSystemInDarkTheme()) darkColorScheme() else lightColorScheme() - MaterialTheme(colorScheme = colorScheme) { - Surface { - VoiceRecordingLockFab(visible = true, offsetY = PREVIEW_DRAG_OFFSET_PX) - } - } -} - -@Preview(name = "Hidden · Light") -@Composable -private fun HiddenPreview() { - MaterialTheme(colorScheme = lightColorScheme()) { - Surface { - VoiceRecordingLockFab(visible = false, offsetY = 0f) - } - } -} From fb4d77d1af63ac5867880be2c176de58c8d35b8a Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Mon, 10 Aug 2026 16:27:55 +0200 Subject: [PATCH 20/27] fix(chat): don't merge closeBacklog's fallback range with the backlog rounds' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closeBacklog's fallback fires when a backlog exceeds MAX_BACKLOG_ROUNDS worth of fetches and switches to pulling the newest messages directly. That fallback lands in its own, disconnected chat block (see updateBlocks) — it is explicitly not contiguous with whatever the backlog rounds already fetched. The returned SyncOutcome nevertheless combined both: oldestPersisted from the backlog rounds with the fallback's own newest id, and summed both counts. This reported a misleadingly contiguous oldest..newest span for two unrelated ranges with an unclosed gap in between — currently only surfacing in a log line, but a footgun for any future caller that assumes the range is coherent. Report the fallback's own oldest/newest/count instead, and fold the backlog rounds' totals into the existing warning log so that information isn't lost, just no longer misattributed to a single range. persistedNewMessages stays an aggregate (totalCount > 0 || fallbackOutcome.persistedNewMessages): it only answers "was anything new persisted in this call", which holds independently of whether the two ranges are contiguous. Reducing it to fallbackOutcome.persistedNewMessages would incorrectly report "nothing new" whenever the terminal fallback fetch itself finds nothing further, even though the backlog rounds already persisted real messages moments earlier in the same call — which would, for example, make fetchNewMessages()'s caller retry a sync that had already succeeded. Update the closeBacklog fallback test accordingly. Assisted-by: Claude Sonnet 5 Signed-off-by: Marcel Hibbe --- .../talk/chat/data/network/ChatMessageSyncer.kt | 9 +++++---- .../talk/chat/data/network/ChatMessageSyncerTest.kt | 6 ++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index f74c860ec47..c28a06f5a19 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -413,7 +413,8 @@ class ChatMessageSyncer @Inject constructor( Log.w( TAG, "Backlog above $fromMessageId in ${target.internalConversationId} still not closed after " + - "$MAX_BACKLOG_ROUNDS rounds, fetching the newest messages instead" + "$MAX_BACKLOG_ROUNDS rounds (persisted $totalCount message(s), ids " + + "$oldestPersisted..$newestPersisted), fetching the newest messages instead" ) val fallbackOutcome = pullAndPersistMessages( target, @@ -431,9 +432,9 @@ class ChatMessageSyncer @Inject constructor( ) return SyncOutcome( persistedNewMessages = totalCount > 0 || fallbackOutcome.persistedNewMessages, - newestPersistedMessageId = fallbackOutcome.newestPersistedMessageId ?: newestPersisted, - oldestPersistedMessageId = oldestPersisted ?: fallbackOutcome.oldestPersistedMessageId, - persistedMessageCount = totalCount + fallbackOutcome.persistedMessageCount + newestPersistedMessageId = fallbackOutcome.newestPersistedMessageId, + oldestPersistedMessageId = fallbackOutcome.oldestPersistedMessageId, + persistedMessageCount = fallbackOutcome.persistedMessageCount ) } diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 53247ab87fe..620fce64ecc 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -339,9 +339,11 @@ class ChatMessageSyncerTest { val outcome = syncer.closeBacklog(target(), fromMessageId = 42, limit = 1) + // the reported range is the fallback's own — it must not be merged with the backlog + // rounds' range, since the fallback lands in a separate, disconnected chat block assertTrue(outcome.persistedNewMessages) - assertEquals(6, outcome.persistedMessageCount) - assertEquals(43L, outcome.oldestPersistedMessageId) + assertEquals(1, outcome.persistedMessageCount) + assertEquals(100L, outcome.oldestPersistedMessageId) assertEquals(100L, outcome.newestPersistedMessageId) val fieldMapCaptor = argumentCaptor>() From d40d3c9fdc2e4a12821fc2a74ef7d3f937283fa2 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 11 Aug 2026 14:58:20 +0200 Subject: [PATCH 21/27] fix(chat): don't skip the initial backlog fetch on a stale lastMessage loadInitialMessages skipped the network fetch entirely whenever the local chat block already reached conversationModel.lastMessage.id, trusting that value as proof we were caught up with the server. That field is only as fresh as the last room list sync, though: a message sent while the app had the conversation list open but the chat closed (and no push-triggered catch-up ran, e.g. on flavors without FCM) never updates it. Opening the chat then wrongly concluded there was nothing to fetch, leaving the newest message missing until whatever live-update mechanism happened to be active caught up on its own. Always close the backlog from the newest locally known message instead of gating on the conversation's cached lastMessage. closeBacklog is a single cheap request when there is genuinely nothing new, so there is no upside to trusting a value that isn't guaranteed current. Drop ChatMessageSyncer.seedHttpSyncedMessageId with it: it existed only to seed the insurance anchor from that same stale field for the now-removed branch, and has no other caller. Assisted-by: Claude Sonnet 5 Signed-off-by: Marcel Hibbe --- .../chat/data/network/ChatMessageSyncer.kt | 11 +------ .../network/OfflineFirstChatRepository.kt | 29 ++++--------------- .../data/network/ChatMessageSyncerTest.kt | 10 ------- 3 files changed, 7 insertions(+), 43 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index c28a06f5a19..2a0634071f2 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -291,20 +291,11 @@ class ChatMessageSyncer @Inject constructor( /** * The newest message id of the conversation/thread confirmed via HTTP sync, or null when no - * sync happened yet (and [seedHttpSyncedMessageId] was never called) since app start. + * sync happened yet since app start. */ fun lastHttpSyncedMessageId(internalConversationId: String, threadId: Long?): Long? = lastHttpSyncedMessageIds[syncStateKey(internalConversationId, threadId)] - /** - * Seeds the HTTP-synced anchor without a fetch. Only call with ids whose coverage is proven by - * HTTP-derived data — e.g. the conversation's lastMessage from the room list sync when the - * local chat block already reaches it. The anchor only ever moves forward. - */ - fun seedHttpSyncedMessageId(internalConversationId: String, threadId: Long?, messageId: Long) { - lastHttpSyncedMessageIds.merge(syncStateKey(internalConversationId, threadId), messageId, ::maxOf) - } - private fun recordHttpSyncedMessageId(target: SyncTarget, messageId: Long) { lastHttpSyncedMessageIds.merge(syncStateKey(target.internalConversationId, target.threadId), messageId, ::maxOf) } diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index dfb35b608c3..1600d3cabaa 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -188,33 +188,10 @@ class OfflineFirstChatRepository @Inject constructor( val weAlreadyHaveSomeOfflineMessages = newestMessageIdFromDb > 0 val weHaveAtLeastTheLastReadMessage = newestMessageIdFromDb >= conversationModel.lastReadMessage.toLong() - val lastMessageIdFromServer = conversationModel.lastMessage?.id ?: 0 - val weHaveTheLastMessage = newestMessageIdFromDb >= lastMessageIdFromServer Log.d(TAG, "weAlreadyHaveSomeOfflineMessages:$weAlreadyHaveSomeOfflineMessages") Log.d(TAG, "weHaveAtLeastTheLastReadMessage:$weHaveAtLeastTheLastReadMessage") - Log.d(TAG, "weHaveTheLastMessage:$weHaveTheLastMessage (lastMessageIdFromServer:$lastMessageIdFromServer)") when { - weAlreadyHaveSomeOfflineMessages && weHaveTheLastMessage -> { - // The offline messages already reach the conversation's last message (e.g. because - // the room list sync prefetched them), so no initial request is needed at all — - // regardless of the live-update mode. Anything newer is handled by long polling, - // the chat relay or the insurance requests. - Log.d( - TAG, - "Initial online request is skipped because offline messages are up to date" + - " until the conversation's last message" - ) - - // No HTTP fetch happens in this branch, so seed the insurance anchor explicitly: - // lastMessage came from the room list sync (HTTP) and the local chat block reaches - // it, so it is a valid HTTP-synced anchor. Without the seed, the first insurance - // request of this session would query with lastKnownMessageId=0. - conversationModel.lastMessage?.id?.let { - syncer.seedHttpSyncedMessageId(internalConversationId, threadId, it) - } - } - weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage -> { // Close the backlog since the newest offline message. This is required on // chat-relay servers (the relay cannot deliver messages that arrived while the @@ -223,6 +200,12 @@ class OfflineFirstChatRepository @Inject constructor( // initial load never has to know the live-update mode, i.e. it must not wait for // the websocket. closeBacklog loops until the backlog is fully closed — a single // capped fetch could leave a permanent gap on chat-relay servers. + // + // This request is always made, even if the conversation's cached lastMessage + // suggests we are already caught up: that value is only as fresh as the last room + // list sync and can already be behind the server by the time the chat is opened. + // closeBacklog is cheap when there is truly nothing new (a single request coming + // back empty), so there is no good reason to trust the stale value instead. Log.d(TAG, "Closing the backlog from the newest offline message for initial loading") syncer.closeBacklog( diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 620fce64ecc..ae4dde17dba 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -402,16 +402,6 @@ class ChatMessageSyncerTest { assertEquals(42L, syncer.lastHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null)) } - @Test - fun `seeded insurance anchor only moves forward`() { - assertNull(syncer.lastHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null)) - - syncer.seedHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null, 44L) - syncer.seedHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null, 10L) - - assertEquals(44L, syncer.lastHttpSyncedMessageId(INTERNAL_CONVERSATION_ID, null)) - } - @Test fun `cleanupExpiredMessages trims block boundaries and deletes empty blocks`() = runTest { From 36942f3e92d5d9028d4c9be26716292c1f8b83e5 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 11 Aug 2026 16:28:41 +0200 Subject: [PATCH 22/27] refactor(chat): extract loadInitialMessages branches into named functions Turn the two-way choice in loadInitialMessages into a plain if/else on a single named condition, weLikelyOnlyHaveASmallBacklog, and extract each branch body into its own function: closeBacklogFromNewestOfflineMessage and fetchNewestMessagesForInitialLoad. No behavior change. closeBacklogFromNewestOfflineMessage's fallback-safety rationale and fetchNewestMessagesForInitialLoad's per-case log messages are now documented against the actual booleans they depend on, rather than being inferred from which when-branch happened to call them. Assisted-by: Claude Sonnet 5 Signed-off-by: Marcel Hibbe --- .../network/OfflineFirstChatRepository.kt | 103 +++++++++--------- 1 file changed, 53 insertions(+), 50 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 1600d3cabaa..21ceba67723 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -173,7 +173,6 @@ class OfflineFirstChatRepository @Inject constructor( } } - @Suppress("LongMethod") override suspend fun loadInitialMessages(withNetworkParams: Bundle) { logger.d(TAG, "---- loadInitialMessages ------------") cleanupExpiredMessages() @@ -186,64 +185,68 @@ class OfflineFirstChatRepository @Inject constructor( Log.d(TAG, "newestMessageIdFromDb: $newestMessageIdFromDb") val weAlreadyHaveSomeOfflineMessages = newestMessageIdFromDb > 0 - val weHaveAtLeastTheLastReadMessage = newestMessageIdFromDb >= conversationModel.lastReadMessage.toLong() + val weLikelyOnlyHaveASmallBacklog = weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage + Log.d(TAG, "weAlreadyHaveSomeOfflineMessages:$weAlreadyHaveSomeOfflineMessages") Log.d(TAG, "weHaveAtLeastTheLastReadMessage:$weHaveAtLeastTheLastReadMessage") + Log.d(TAG, "weLikelyOnlyHaveASmallBacklog:$weLikelyOnlyHaveASmallBacklog") - when { - weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage -> { - // Close the backlog since the newest offline message. This is required on - // chat-relay servers (the relay cannot deliver messages that arrived while the - // app was closed) and is equally cheap on long-polling servers, where it just - // front-loads what the first poll request would have fetched. This way the - // initial load never has to know the live-update mode, i.e. it must not wait for - // the websocket. closeBacklog loops until the backlog is fully closed — a single - // capped fetch could leave a permanent gap on chat-relay servers. - // - // This request is always made, even if the conversation's cached lastMessage - // suggests we are already caught up: that value is only as fresh as the last room - // list sync and can already be behind the server by the time the chat is opened. - // closeBacklog is cheap when there is truly nothing new (a single request coming - // back empty), so there is no good reason to trust the stale value instead. - Log.d(TAG, "Closing the backlog from the newest offline message for initial loading") - - syncer.closeBacklog( - target = syncTarget, - fromMessageId = newestMessageIdFromDb, - lastCommonRead = newXChatLastCommonRead, - events = syncEvents - ) - } + if (weLikelyOnlyHaveASmallBacklog) { + closeBacklogFromNewestOfflineMessage(newestMessageIdFromDb) + } else { + fetchNewestMessagesForInitialLoad( + withNetworkParams, + weAlreadyHaveSomeOfflineMessages, + weHaveAtLeastTheLastReadMessage + ) + } + } - else -> { - if (!weAlreadyHaveSomeOfflineMessages) { - Log.d(TAG, "An online request for newest 100 messages is made because offline chat is empty") - if (networkMonitor.isOnline.value.not()) { - // _generalUIFlow.emit(ChatActivity.NO_OFFLINE_MESSAGES_FOUND) - } - } else { - Log.d( - TAG, - "An online request for newest 100 messages is made because we don't have the " + - "lastReadMessage (gaps could be closed by scrolling up to merge the chatblocks)" - ) - } + /** + * Tries to close the backlog since the newest offline message. + */ + private suspend fun closeBacklogFromNewestOfflineMessage(newestMessageIdFromDb: Long) { + Log.d(TAG, "Closing the backlog from the newest offline message for initial loading") - // set up field map to load the newest messages - val fieldMap = getFieldMap( - lookIntoFuture = false, - timeout = 0, - includeLastKnown = true, - lastKnown = null - ) - withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) - withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token) + syncer.closeBacklog( + target = syncTarget, + fromMessageId = newestMessageIdFromDb, + lastCommonRead = newXChatLastCommonRead, + events = syncEvents + ) + } - Log.d(TAG, "Starting online request for initial loading") - getAndPersistMessages(withNetworkParams) + private suspend fun fetchNewestMessagesForInitialLoad( + withNetworkParams: Bundle, + weAlreadyHaveSomeOfflineMessages: Boolean, + weHaveAtLeastTheLastReadMessage: Boolean + ) { + if (!weAlreadyHaveSomeOfflineMessages) { + Log.d(TAG, "An online request for newest 100 messages is made because offline chat is empty") + if (networkMonitor.isOnline.value.not()) { + // _generalUIFlow.emit(ChatActivity.NO_OFFLINE_MESSAGES_FOUND) } + } else if (!weHaveAtLeastTheLastReadMessage) { + Log.d( + TAG, + "An online request for newest 100 messages is made because we don't have the " + + "lastReadMessage (gaps could be closed by scrolling up to merge the chatblocks)" + ) } + + // set up field map to load the newest messages + val fieldMap = getFieldMap( + lookIntoFuture = false, + timeout = 0, + includeLastKnown = true, + lastKnown = null + ) + withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) + withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token) + + Log.d(TAG, "Starting online request for initial loading") + getAndPersistMessages(withNetworkParams) } override suspend fun startMessagePolling(hasHighPerformanceBackend: Boolean) { From a78dd14dc8d0a77629ecb8ccabeabcaabf828aff Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 11 Aug 2026 16:42:02 +0200 Subject: [PATCH 23/27] refactor(chat): rename closeBacklog to tryCloseBacklog closeBacklog can fall back to fetching the newest messages instead of actually closing the gap once MAX_BACKLOG_ROUNDS is exceeded, leaving the remaining range genuinely open for later. The name promised full closure with no such caveat. Rename it and its OfflineFirstChatRepository wrapper (closeBacklogFromNewestOfflineMessage) to make the best-effort nature explicit, and update call sites, comments, and test names to match. Assisted-by: Claude Sonnet 5 Signed-off-by: Marcel Hibbe --- .../talk/chat/data/network/ChatMessageSyncer.kt | 9 +++++---- .../chat/data/network/OfflineFirstChatRepository.kt | 12 ++++++------ .../talk/chat/data/network/ChatMessageSyncerTest.kt | 8 ++++---- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 2a0634071f2..0591d6a0382 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -308,7 +308,7 @@ class ChatMessageSyncer @Inject constructor( chatBlocksDao.getNewestMessageIdFromChatBlocks(target.internalConversationId, target.threadId) val outcome = if (newestMessageIdFromDb > 0) { - closeBacklog( + tryCloseBacklog( target = target, fromMessageId = newestMessageIdFromDb, limit = limit, @@ -344,7 +344,8 @@ class ChatMessageSyncer @Inject constructor( } /** - * Fetches the messages newer than [fromMessageId] until the backlog is fully closed. + * Tries to fetch the messages newer than [fromMessageId] until the backlog is fully closed — + * there is no guarantee it will be: see the [MAX_BACKLOG_ROUNDS] fallback below. * * A single fetch is capped by [limit], so one request only narrows a backlog larger than * that — and on chat-relay servers the remaining gap would become permanent as soon as a @@ -355,8 +356,8 @@ class ChatMessageSyncer @Inject constructor( * stays visible in the block structure (closable by scrolling up) rather than a block * claiming ranges that were never fetched. */ - @Suppress("LongParameterList") - suspend fun closeBacklog( + @Suppress("LongParameterList", "LongMethod") + suspend fun tryCloseBacklog( target: SyncTarget, fromMessageId: Long, limit: Int = DEFAULT_MESSAGES_LIMIT, diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 21ceba67723..8ac91288940 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -193,7 +193,7 @@ class OfflineFirstChatRepository @Inject constructor( Log.d(TAG, "weLikelyOnlyHaveASmallBacklog:$weLikelyOnlyHaveASmallBacklog") if (weLikelyOnlyHaveASmallBacklog) { - closeBacklogFromNewestOfflineMessage(newestMessageIdFromDb) + tryCloseBacklogFromNewestOfflineMessage(newestMessageIdFromDb) } else { fetchNewestMessagesForInitialLoad( withNetworkParams, @@ -206,10 +206,10 @@ class OfflineFirstChatRepository @Inject constructor( /** * Tries to close the backlog since the newest offline message. */ - private suspend fun closeBacklogFromNewestOfflineMessage(newestMessageIdFromDb: Long) { - Log.d(TAG, "Closing the backlog from the newest offline message for initial loading") + private suspend fun tryCloseBacklogFromNewestOfflineMessage(newestMessageIdFromDb: Long) { + Log.d(TAG, "Try to close the backlog from the newest offline message for initial loading") - syncer.closeBacklog( + syncer.tryCloseBacklog( target = syncTarget, fromMessageId = newestMessageIdFromDb, lastCommonRead = newXChatLastCommonRead, @@ -338,10 +338,10 @@ class OfflineFirstChatRepository @Inject constructor( val lastHttpSyncedMessageId = syncer.lastHttpSyncedMessageId(internalConversationId, threadId) ?: 0L - // closeBacklog loops until the backlog is fully closed, so a backlog larger than the + // tryCloseBacklog loops until the backlog is fully closed, so a backlog larger than the // request limit is caught up within one insurance cycle instead of narrowing it by one // page every cycle. - val outcome = syncer.closeBacklog( + val outcome = syncer.tryCloseBacklog( target = syncTarget, fromMessageId = lastHttpSyncedMessageId, limit = 200, diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index ae4dde17dba..990f5e35ab6 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -293,7 +293,7 @@ class ChatMessageSyncerTest { } @Test - fun `closeBacklog loops until the server returns fewer messages than the limit`() = + fun `tryCloseBacklog loops until the server returns fewer messages than the limit`() = runTest { val existingBlock = block(oldest = 10, newest = 42) whenever(chatBlocksDao.getChatBlocksContainingMessageId(eq(INTERNAL_CONVERSATION_ID), eq(null), any())) @@ -306,7 +306,7 @@ class ChatMessageSyncerTest { Response.success(overall(message(45))) ) - val outcome = syncer.closeBacklog(target(), fromMessageId = 42, limit = 2) + val outcome = syncer.tryCloseBacklog(target(), fromMessageId = 42, limit = 2) assertTrue(outcome.persistedNewMessages) assertEquals(3, outcome.persistedMessageCount) @@ -320,7 +320,7 @@ class ChatMessageSyncerTest { } @Test - fun `closeBacklog falls back to the newest messages when the backlog persists`() = + fun `tryCloseBacklog falls back to the newest messages when the backlog persists`() = runTest { val existingBlock = block(oldest = 10, newest = 42) whenever(chatBlocksDao.getChatBlocksContainingMessageId(eq(INTERNAL_CONVERSATION_ID), eq(null), any())) @@ -337,7 +337,7 @@ class ChatMessageSyncerTest { Response.success(overall(message(100))) ) - val outcome = syncer.closeBacklog(target(), fromMessageId = 42, limit = 1) + val outcome = syncer.tryCloseBacklog(target(), fromMessageId = 42, limit = 1) // the reported range is the fallback's own — it must not be merged with the backlog // rounds' range, since the fallback lands in a separate, disconnected chat block From dbdf2b5054aa94877df285b9704e7255531701e0 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 13 Aug 2026 10:06:54 +0200 Subject: [PATCH 24/27] fix(chat): seed read receipts from the conversation on chat open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With prefetching, a chat usually opens with an already up-to-date cache, so every pull request sends the server's own lastCommonReadId back and is answered with 304 Not Modified. The X-Chat-Last-Common-Read header only exists on 200 responses, so _lastCommonReadFlow never emitted and the UI stayed at its initial 0 — every own message rendered with a single checkmark regardless of its actual read state. - seed _lastCommonReadFlow directly in loadInitialMessages: the conversation entity is kept fresh by the room list sync and the prefetch, so its lastCommonReadMessage is the correct initial value - give _lastCommonReadFlow replay = 1 so the seed survives subscriber timing between repository and view model - keep the previous value when a 200 response carries no X-Chat-Last-Common-Read header: overwriting with null dropped lastCommonReadId from all following field maps, and without that parameter the server never reports pure read-state changes Known limitation (follow-up): while a chat is open against a high performance backend, read-state changes still only arrive with the insurance requests (every 2 minutes), because signaling messages carry no read state and 304 responses carry no header. The room list sync refreshes the conversation's lastCommonReadMessage far more often, but ChatViewModel's conversationFlow is filtered with distinctUntilChangedBy { it.lastReadMessage }, so that fresher value never reaches the message list. Folding it in — e.g. maxOf(repository value, conversation.lastCommonReadMessage) in observeMessages — requires relaxing that filter and should be done as a separate change. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../talk/chat/data/network/OfflineFirstChatRepository.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 8ac91288940..01e0187bdc6 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -90,7 +90,7 @@ class OfflineFirstChatRepository @Inject constructor( get() = _lastCommonReadFlow private val _lastCommonReadFlow: - MutableSharedFlow = MutableSharedFlow() + MutableSharedFlow = MutableSharedFlow(replay = 1) override val lastReadMessageFlow: Flow get() = _lastReadMessageFlow @@ -156,7 +156,7 @@ class OfflineFirstChatRepository @Inject constructor( private val syncEvents = object : ChatMessageSyncer.Events { override suspend fun onLastCommonReadChanged(lastCommonRead: Int?) { - newXChatLastCommonRead = lastCommonRead + newXChatLastCommonRead = lastCommonRead ?: newXChatLastCommonRead updateUiForLastCommonRead() } @@ -177,6 +177,7 @@ class OfflineFirstChatRepository @Inject constructor( logger.d(TAG, "---- loadInitialMessages ------------") cleanupExpiredMessages() newXChatLastCommonRead = conversationModel.lastCommonReadMessage + updateUiForLastCommonRead() Log.d(TAG, "conversationModel.internalId: " + conversationModel.internalId) Log.d(TAG, "conversationModel.lastReadMessage:" + conversationModel.lastReadMessage) From 61ba4e8a68c98842edc178d607e897fe450020f6 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 13 Aug 2026 10:15:23 +0200 Subject: [PATCH 25/27] fix(chat): update read receipts from the conversation while a chat is open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read-receipt checkmarks were only updated by the X-Chat-Last-Common-Read header of 200 chat pull responses. On a high performance backend those are rare while a chat is open: messages arrive via signaling (which carries no read state) and the insurance requests only run every two minutes, so a pure read-state change took up to two minutes to show — even though the room list sync keeps writing a fresher lastCommonReadMessage into the conversation row all along. - let conversationFlow re-emit when lastCommonReadMessage changes instead of filtering on lastReadMessage alone - fold the conversation's value into the checkmark input with maxOf(header value, conversation.lastCommonReadMessage): either side can be ahead of the other, and since lastCommonRead only moves forward server-side the maximum is always correct The extra re-emissions are harmless: the message list rebuild is debounced, observeConversation is idempotent, conversationAndUserFlow is consumed with take(1) and the pinned-message flow deduplicates downstream. Also removes a left-over debug println from conversationFlow. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index e5b7e8978e5..996346fe5d3 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -483,10 +483,7 @@ class ChatViewModel @AssistedInject constructor( null } } - .distinctUntilChangedBy { it.lastReadMessage } - .onEach { - println("Conversation changed: lastRead=${it.lastReadMessage}") - } + .distinctUntilChangedBy { it.lastReadMessage to it.lastCommonReadMessage } private val conversationAndUserFlow = combine(conversationFlow, nonNullUserFlow) { c, u -> c to u } @@ -1057,7 +1054,7 @@ class ChatViewModel @AssistedInject constructor( val (conversation, capabilities) = convAndCaps CombinedInput( messages, - lastCommonRead, + maxOf(lastCommonRead, conversation.lastCommonReadMessage), parentMap, conversation.lastReadMessage, expandedParents, From ea40bb07fbf4e281062bda70128c07537afcf06e Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 13 Aug 2026 12:13:56 +0200 Subject: [PATCH 26/27] refactor(chat): move push message catch-up into a dedicated worker Fetching the pushed room's messages no longer runs synchronously inside NotificationWorker. It now enqueues a ChatMessageCatchUpWorker (network-constrained, exponential backoff) after the notification is displayed, so a slow or failing fetch can never delay the notification and transient failures are retried instead of lost. SyncOutcome gained a syncFailed flag so the worker can tell "no new messages" from a failed fetch. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../chat/data/network/ChatMessageSyncer.kt | 20 ++- .../talk/jobs/ChatMessageCatchUpWorker.kt | 134 ++++++++++++++++++ .../nextcloud/talk/jobs/NotificationWorker.kt | 45 ++---- app/src/test/java/android/util/Log.kt | 7 + .../data/network/ChatMessageSyncerTest.kt | 20 ++- 5 files changed, 183 insertions(+), 43 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/jobs/ChatMessageCatchUpWorker.kt diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 0591d6a0382..9e9b891bbfc 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -87,11 +87,17 @@ 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. + */ data class SyncOutcome( val persistedNewMessages: Boolean, val newestPersistedMessageId: Long?, val oldestPersistedMessageId: Long? = null, - val persistedMessageCount: Int = 0 + val persistedMessageCount: Int = 0, + val syncFailed: Boolean = false ) /** @@ -211,7 +217,7 @@ class ChatMessageSyncer @Inject constructor( when { !networkMonitor.isOnline.value -> { Log.d(TAG, "Device is offline, skipping catch-up for ${target.internalConversationId}") - NOTHING_SYNCED + SYNC_FAILED } !target.user.hasSpreedFeatureCapability(SpreedFeatures.CHAT_KEEP_NOTIFICATIONS.value) -> { @@ -396,7 +402,8 @@ class ChatMessageSyncer @Inject constructor( persistedNewMessages = totalCount > 0, newestPersistedMessageId = newestPersisted, oldestPersistedMessageId = oldestPersisted, - persistedMessageCount = totalCount + persistedMessageCount = totalCount, + syncFailed = roundOutcome.syncFailed ) } anchor = nextAnchor @@ -426,7 +433,8 @@ class ChatMessageSyncer @Inject constructor( persistedNewMessages = totalCount > 0 || fallbackOutcome.persistedNewMessages, newestPersistedMessageId = fallbackOutcome.newestPersistedMessageId, oldestPersistedMessageId = fallbackOutcome.oldestPersistedMessageId, - persistedMessageCount = fallbackOutcome.persistedMessageCount + persistedMessageCount = fallbackOutcome.persistedMessageCount, + syncFailed = fallbackOutcome.syncFailed ) } @@ -507,7 +515,7 @@ class ChatMessageSyncer @Inject constructor( is ChatPullResult.Error -> { Log.e(TAG, "Error pulling messages from server", result.throwable) - NOTHING_SYNCED + SYNC_FAILED } } } finally { @@ -869,6 +877,8 @@ class ChatMessageSyncer @Inject constructor( val NO_EVENTS: Events = object : Events {} private val NOTHING_SYNCED = SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null) + private val SYNC_FAILED = + SyncOutcome(persistedNewMessages = false, newestPersistedMessageId = null, syncFailed = true) private const val DEFAULT_MESSAGES_LIMIT = 100 private const val MILLIS_PER_SECOND = 1000L diff --git a/app/src/main/java/com/nextcloud/talk/jobs/ChatMessageCatchUpWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/ChatMessageCatchUpWorker.kt new file mode 100644 index 00000000000..97314055021 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/jobs/ChatMessageCatchUpWorker.kt @@ -0,0 +1,134 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Andy Scherzinger + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.jobs + +import android.content.Context +import android.os.PowerManager +import android.util.Log +import androidx.work.BackoffPolicy +import androidx.work.Constraints +import androidx.work.CoroutineWorker +import androidx.work.Data +import androidx.work.NetworkType +import androidx.work.OneTimeWorkRequest +import androidx.work.WorkManager +import androidx.work.WorkRequest +import androidx.work.WorkerParameters +import autodagger.AutoInjector +import com.nextcloud.talk.application.NextcloudTalkApplication +import com.nextcloud.talk.application.NextcloudTalkApplication.Companion.sharedApplication +import com.nextcloud.talk.chat.data.network.ChatMessageSyncer +import com.nextcloud.talk.users.UserManager +import com.nextcloud.talk.utils.ApiUtils +import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_INTERNAL_USER_ID +import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN +import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_THREAD_ID +import java.util.concurrent.TimeUnit +import javax.inject.Inject + +/** + * Prefetches a pushed room's messages into the local database so they are instantly visible when + * the chat is opened from the notification (or later). Enqueued by [NotificationWorker] after the + * notification is displayed, so the fetch never delays or suppresses the notification and + * transient failures are retried with backoff instead of being lost with the notification worker. + * + * Push bursts for the same room enqueue one worker each, but the per-room coalescing of the + * [ChatMessageSyncer] singleton collapses overlapping catch-ups into few actual fetches. Skipped + * in battery saver mode; the chat-keep-notifications capability gate and the offline check are + * handled inside [ChatMessageSyncer.catchUpRoom]. + */ +@AutoInjector(NextcloudTalkApplication::class) +class ChatMessageCatchUpWorker(context: Context, workerParams: WorkerParameters) : + CoroutineWorker(context, workerParams) { + + @Inject + lateinit var userManager: UserManager + + @Inject + lateinit var chatMessageSyncer: ChatMessageSyncer + + override suspend fun doWork(): Result { + sharedApplication!!.componentApplication.inject(this) + + val userId = inputData.getLong(KEY_INTERNAL_USER_ID, -1) + val roomToken = inputData.getString(KEY_ROOM_TOKEN) + val threadId = inputData.getLong(KEY_THREAD_ID, NO_THREAD).takeIf { it != NO_THREAD } + + return when { + userId < 0 || roomToken.isNullOrEmpty() -> { + Log.e(TAG, "Missing user id or room token, dropping message catch-up") + Result.failure() + } + + isPowerSaveMode() -> { + Log.d(TAG, "Battery saver is active, skipping message catch-up for room $roomToken") + Result.success() + } + + else -> catchUpRoom(userId, roomToken, threadId) + } + } + + private suspend fun catchUpRoom(userId: Long, roomToken: String, threadId: Long?): Result { + val user = userManager.getUserWithId(userId).blockingGet() + val credentials = user?.let { ApiUtils.getCredentials(it.username, it.token) } + if (user == null || credentials == null) { + Log.e(TAG, "No user or credentials found for user id $userId, dropping message catch-up") + return Result.failure() + } + + val target = ChatMessageSyncer.SyncTarget( + user = user, + roomToken = roomToken, + threadId = threadId, + credentials = credentials, + urlForChatting = ApiUtils.getUrlForChat(CHAT_API_VERSION, user.baseUrl!!, roomToken) + ) + + val outcome = runCatching { chatMessageSyncer.catchUpRoom(target) }.getOrElse { throwable -> + Log.e(TAG, "Message catch-up failed for room $roomToken", throwable) + null + } + + return if (outcome == null || outcome.syncFailed) { + Log.w(TAG, "Message catch-up for room $roomToken did not complete (attempt ${runAttemptCount + 1})") + retryOrFail() + } else { + Result.success() + } + } + + private fun retryOrFail(): Result = if (runAttemptCount < MAX_RUN_ATTEMPTS - 1) Result.retry() else Result.failure() + + private fun isPowerSaveMode(): Boolean { + val powerManager = applicationContext.getSystemService(Context.POWER_SERVICE) as PowerManager + return powerManager.isPowerSaveMode + } + + companion object { + private val TAG: String = ChatMessageCatchUpWorker::class.java.simpleName + private const val CHAT_API_VERSION = 1 + private const val NO_THREAD = -1L + private const val MAX_RUN_ATTEMPTS = 3 + + fun enqueue(context: Context, userId: Long, roomToken: String, threadId: Long?) { + val data = Data.Builder() + .putLong(KEY_INTERNAL_USER_ID, userId) + .putString(KEY_ROOM_TOKEN, roomToken) + .apply { threadId?.let { putLong(KEY_THREAD_ID, it) } } + .build() + + val catchUpWork = OneTimeWorkRequest.Builder(ChatMessageCatchUpWorker::class.java) + .setInputData(data) + .setConstraints(Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build()) + .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, WorkRequest.MIN_BACKOFF_MILLIS, TimeUnit.MILLISECONDS) + .build() + + WorkManager.getInstance(context).enqueue(catchUpWork) + } + } +} diff --git a/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt index 4838b5c1be4..6ea912878cd 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt @@ -20,7 +20,6 @@ import android.os.Build import android.os.Bundle import android.os.Handler import android.os.Looper -import android.os.PowerManager import android.os.SystemClock import android.service.notification.StatusBarNotification import android.text.TextUtils @@ -53,7 +52,6 @@ import com.nextcloud.talk.application.NextcloudTalkApplication import com.nextcloud.talk.application.NextcloudTalkApplication.Companion.sharedApplication import com.nextcloud.talk.arbitrarystorage.ArbitraryStorageManager import com.nextcloud.talk.callnotification.CallNotificationActivity -import com.nextcloud.talk.chat.data.network.ChatMessageSyncer import com.nextcloud.talk.chat.data.network.ChatNetworkDataSource import com.nextcloud.talk.conversationlist.DirectShareHelper import com.nextcloud.talk.data.user.model.User @@ -138,9 +136,6 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor var chatNetworkDataSource: ChatNetworkDataSource? = null @Inject set - var chatMessageSyncer: ChatMessageSyncer? = null - @Inject set - @Inject lateinit var userManager: UserManager @@ -219,42 +214,19 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor } /** - * Prefetches the pushed room's messages into the local database so they are instantly visible - * when the chat is opened from the notification (or later). For messages in a thread, - * [threadId] targets the thread so its chat block is extended. Best effort only: failures are - * logged and never delay or suppress the notification, which is displayed beforehand. - * Skipped in battery saver mode; the chat-keep-notifications capability gate and the offline - * check are handled inside [ChatMessageSyncer.catchUpRoom]. + * Enqueues a [ChatMessageCatchUpWorker] that prefetches the pushed room's messages into the + * local database so they are instantly visible when the chat is opened from the notification + * (or later). For messages in a thread, [threadId] targets the thread so its chat block is + * extended. The worker runs independently of this one: the notification is displayed + * beforehand and a slow or failing fetch (retried there with backoff) can never delay it. */ private fun catchUpPushedRoom(threadId: Long?) { val roomToken = pushMessage.id - val syncer = chatMessageSyncer - if (pushMessage.type != TYPE_CHAT || roomToken == null || syncer == null || isPowerSaveMode()) { - logger.d(TAG, "Skipping message catch-up for pushed room (not a chat push or battery saver active)") + if (pushMessage.type != TYPE_CHAT || roomToken == null) { + logger.d(TAG, "Skipping message catch-up for pushed room (not a chat push)") return } - - // the user from the push signature verification may carry stale capabilities, so resolve - // the current state before the capability check in catchUpRoom - val currentUser = userManager.getUserWithId(user.id!!).blockingGet() ?: return - - val target = ChatMessageSyncer.SyncTarget( - user = currentUser, - roomToken = roomToken, - threadId = threadId, - credentials = credentials, - urlForChatting = ApiUtils.getUrlForChat(CHAT_API_VERSION, currentUser.baseUrl!!, roomToken) - ) - runCatching { - runBlocking { syncer.catchUpRoom(target) } - }.onFailure { - Log.e(TAG, "Message catch-up after push failed for room $roomToken", it) - } - } - - private fun isPowerSaveMode(): Boolean { - val powerManager = applicationContext.getSystemService(Context.POWER_SERVICE) as PowerManager - return powerManager.isPowerSaveMode + ChatMessageCatchUpWorker.enqueue(applicationContext, user.id!!, roomToken, threadId) } private fun handleRemoteTalkSharePushMessage() { @@ -1250,7 +1222,6 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor companion object { val TAG: String = NotificationWorker::class.java.simpleName private const val TYPE_CHAT = "chat" - private const val CHAT_API_VERSION = 1 private const val TYPE_ROOM = "room" private const val TYPE_CALL = "call" private const val TYPE_RECORDING = "recording" diff --git a/app/src/test/java/android/util/Log.kt b/app/src/test/java/android/util/Log.kt index aa69a10fa32..716ce32a373 100644 --- a/app/src/test/java/android/util/Log.kt +++ b/app/src/test/java/android/util/Log.kt @@ -30,6 +30,13 @@ object Log { return 1 } + @JvmStatic + fun e(tag: String, msg: String, tr: Throwable): Int { + println("ERROR: $tag: $msg: $tr") + + return 1 + } + @JvmStatic fun i(tag: String, msg: String): Int { println("INFO: $tag: $msg") diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 990f5e35ab6..21fd18acfc6 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -95,13 +95,14 @@ class ChatMessageSyncerTest { } @Test - fun `catchUpRoom skips when offline`() = + fun `catchUpRoom skips when offline and marks the sync as failed`() = runTest { whenever(networkMonitor.isOnline).thenReturn(MutableStateFlow(false)) val outcome = syncer.catchUpRoom(target()) assertFalse(outcome.persistedNewMessages) + assertTrue(outcome.syncFailed) verifyNoInteractions(network) } @@ -111,9 +112,25 @@ class ChatMessageSyncerTest { val outcome = syncer.catchUpRoom(target(user(withKeepNotificationsCapability = false))) assertFalse(outcome.persistedNewMessages) + // a missing capability is not retryable, so the skip must not count as failure + assertFalse(outcome.syncFailed) verifyNoInteractions(network) } + @Test + fun `catchUpRoom marks the sync as failed when the server request errors`() = + runTest { + whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null)) + .thenReturn(42L) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.error(HTTP_INTERNAL_SERVER_ERROR, "".toResponseBody())) + + val outcome = syncer.catchUpRoom(target()) + + assertFalse(outcome.persistedNewMessages) + assertTrue(outcome.syncFailed) + } + @Test fun `catchUpRoom delta-fetches from the newest cached message when a chat block exists`() = runTest { @@ -499,5 +516,6 @@ class ChatMessageSyncerTest { private const val CREDENTIALS = "credentials" private const val CHAT_URL = "https://server.example.com/ocs/v2.php/apps/spreed/api/v1/chat/$ROOM_TOKEN" private const val HTTP_NOT_MODIFIED = 304 + private const val HTTP_INTERNAL_SERVER_ERROR = 500 } } From 3f6a24dc694a732657e3a69f34b79b66d453383e Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 13 Aug 2026 12:14:09 +0200 Subject: [PATCH 27/27] fix(chat): merge connected chat blocks atomically The upsert -> getConnectedChatBlocks -> replaceConnectedChatBlocks sequence in ChatMessageSyncer.updateBlocks was not atomic: with the open-path delta, long poll/insurance, signaling and background catch-up all able to update the same conversation concurrently, two callers could each upsert a block and query connectivity before seeing the other's write, leaving overlapping blocks behind. The whole sequence now runs as a single Room @Transaction in ChatBlocksDao.upsertAndMergeConnectedChatBlocks. getConnectedChatBlocks and deleteChatBlocks became suspend so no blocking DAO calls run inside the suspending transaction. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger --- .../data/database/dao/ChatBlocksDaoTest.kt | 121 +++++++++++++++++- .../chat/data/network/ChatMessageSyncer.kt | 47 +------ .../network/OfflineFirstChatRepository.kt | 2 +- .../talk/data/database/dao/ChatBlocksDao.kt | 46 ++++++- .../utils/preview/ComposePreviewUtilsDaos.kt | 6 +- .../data/network/ChatMessageSyncerTest.kt | 36 ++---- 6 files changed, 176 insertions(+), 82 deletions(-) diff --git a/app/src/androidTest/java/com/nextcloud/talk/data/database/dao/ChatBlocksDaoTest.kt b/app/src/androidTest/java/com/nextcloud/talk/data/database/dao/ChatBlocksDaoTest.kt index a838e3cf7df..aed3fe96d7b 100644 --- a/app/src/androidTest/java/com/nextcloud/talk/data/database/dao/ChatBlocksDaoTest.kt +++ b/app/src/androidTest/java/com/nextcloud/talk/data/database/dao/ChatBlocksDaoTest.kt @@ -244,7 +244,7 @@ class ChatBlocksDaoTest { newestMessageId = searchedChatBlock.newestMessageId ) - assertEquals(5, results.first().size) + assertEquals(5, results.size) } @Test @@ -314,7 +314,7 @@ class ChatBlocksDaoTest { newestMessageId = searchedChatBlock.newestMessageId ) - assertEquals(1, resultsForThreadIdNull.first().size) + assertEquals(1, resultsForThreadIdNull.size) val resultsForThreadId123 = chatBlocksDao.getConnectedChatBlocks( internalConversationId = conversation1.internalId, @@ -323,7 +323,7 @@ class ChatBlocksDaoTest { newestMessageId = searchedChatBlock.newestMessageId ) - assertEquals(2, resultsForThreadId123.first().size) + assertEquals(2, resultsForThreadId123.size) } @Test @@ -384,7 +384,7 @@ class ChatBlocksDaoTest { threadId = null, oldestMessageId = 10, newestMessageId = 35 - ).first() + ) assertEquals(3, connectedBlocks.size) val mergedBlock = ChatBlockEntity( @@ -412,10 +412,121 @@ class ChatBlocksDaoTest { threadId = null, oldestMessageId = 10, newestMessageId = 40 - ).first().size + ).size ) } + @Test + fun testUpsertAndMergeConnectedChatBlocksMergesOverlappingBlocks() = + runTest { + val user = createUserEntity("account1", "Account 1") + usersDao.saveUser(user) + val account1 = usersDao.getUserWithUserId("account1").blockingGet() + + conversationsDao.upsertConversations( + account1.id, + listOf( + createConversationEntity( + accountId = account1.id, + token = "abc", + roomName = "Conversation One" + ) + ) + ) + + val conversation = conversationsDao.getConversationsForUser(account1.id).first()[0] + + chatBlocksDao.upsertChatBlock( + ChatBlockEntity( + internalConversationId = conversation.internalId, + accountId = conversation.accountId, + token = conversation.token, + threadId = null, + oldestMessageId = 10, + newestMessageId = 20, + hasHistory = false + ) + ) + chatBlocksDao.upsertChatBlock( + ChatBlockEntity( + internalConversationId = conversation.internalId, + accountId = conversation.accountId, + token = conversation.token, + threadId = null, + oldestMessageId = 25, + newestMessageId = 35, + hasHistory = true + ) + ) + + // the new block overlaps both existing blocks, so all three must merge into one + chatBlocksDao.upsertAndMergeConnectedChatBlocks( + ChatBlockEntity( + internalConversationId = conversation.internalId, + accountId = conversation.accountId, + token = conversation.token, + threadId = null, + oldestMessageId = 18, + newestMessageId = 27, + hasHistory = true + ) + ) + + val blocks = chatBlocksDao.getChatBlocksForConversation(conversation.internalId) + assertEquals(1, blocks.size) + assertEquals(10L, blocks[0].oldestMessageId) + assertEquals(35L, blocks[0].newestMessageId) + assertEquals(false, blocks[0].hasHistory) + } + + @Test + fun testUpsertAndMergeConnectedChatBlocksKeepsDisjointBlocksSeparate() = + runTest { + val user = createUserEntity("account1", "Account 1") + usersDao.saveUser(user) + val account1 = usersDao.getUserWithUserId("account1").blockingGet() + + conversationsDao.upsertConversations( + account1.id, + listOf( + createConversationEntity( + accountId = account1.id, + token = "abc", + roomName = "Conversation One" + ) + ) + ) + + val conversation = conversationsDao.getConversationsForUser(account1.id).first()[0] + + chatBlocksDao.upsertChatBlock( + ChatBlockEntity( + internalConversationId = conversation.internalId, + accountId = conversation.accountId, + token = conversation.token, + threadId = null, + oldestMessageId = 10, + newestMessageId = 20, + hasHistory = true + ) + ) + + chatBlocksDao.upsertAndMergeConnectedChatBlocks( + ChatBlockEntity( + internalConversationId = conversation.internalId, + accountId = conversation.accountId, + token = conversation.token, + threadId = null, + oldestMessageId = 30, + newestMessageId = 40, + hasHistory = true + ) + ) + + val blocks = chatBlocksDao.getChatBlocksForConversation(conversation.internalId) + assertEquals(2, blocks.size) + } + private fun createUserEntity(userId: String, userName: String) = UserEntity( userId = userId, diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 9e9b891bbfc..53509965d87 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -632,7 +632,7 @@ class ChatMessageSyncer @Inject constructor( newestMessageId = newestMessageIdForNewChatBlock, hasHistory = hasHistory ) - updateBlocks(target, newChatBlock) + chatBlocksDao.upsertAndMergeConnectedChatBlocks(newChatBlock) return chatMessageEntities } @@ -826,51 +826,6 @@ class ChatMessageSyncer @Inject constructor( return blockContainingQueriedMessage } - suspend fun updateBlocks(target: SyncTarget, chatBlock: ChatBlockEntity) { - chatBlocksDao.upsertChatBlock(chatBlock) - - val connectedChatBlocks = - chatBlocksDao.getConnectedChatBlocks( - internalConversationId = target.internalConversationId, - threadId = target.threadId, - oldestMessageId = chatBlock.oldestMessageId, - newestMessageId = chatBlock.newestMessageId - ).first() - - if (connectedChatBlocks.size == 1) { - Log.d(TAG, "This chatBlock is not connected to others") - val chatBlockFromDb = connectedChatBlocks[0] - Log.d(TAG, "chatBlockFromDb.oldestMessageId: " + chatBlockFromDb.oldestMessageId) - Log.d(TAG, "chatBlockFromDb.newestMessageId: " + chatBlockFromDb.newestMessageId) - } else if (connectedChatBlocks.size > 1) { - Log.d(TAG, "Found " + connectedChatBlocks.size + " chat blocks that are connected") - val oldestIdFromDbChatBlocks = - connectedChatBlocks.minByOrNull { it.oldestMessageId }!!.oldestMessageId - val newestIdFromDbChatBlocks = - connectedChatBlocks.maxByOrNull { it.newestMessageId }!!.newestMessageId - - val hasNoHistory = connectedChatBlocks.any { !it.hasHistory } - val hasHistory = !hasNoHistory - Log.d(TAG, "hasHistory = $hasHistory") - - val newChatBlock = ChatBlockEntity( - internalConversationId = target.internalConversationId, - accountId = target.accountId, - token = target.roomToken, - threadId = target.threadId, - oldestMessageId = oldestIdFromDbChatBlocks, - newestMessageId = newestIdFromDbChatBlocks, - hasHistory = hasHistory - ) - chatBlocksDao.replaceConnectedChatBlocks(connectedChatBlocks, newChatBlock) - Log.d(TAG, "A new chat block was created that covers all the range of the found chatblocks") - Log.d(TAG, "new chatBlock - oldest MessageId: $oldestIdFromDbChatBlocks") - Log.d(TAG, "new chatBlock - newest MessageId: $newestIdFromDbChatBlocks") - } else { - Log.d(TAG, "No chat block found ....") - } - } - companion object { val TAG: String = ChatMessageSyncer::class.java.simpleName diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 01e0187bdc6..98594e6d856 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -458,7 +458,7 @@ class OfflineFirstChatRepository @Inject constructor( newestMessageId = newestId, hasHistory = true ) - syncer.updateBlocks(syncTarget, block) + chatBlocksDao.upsertAndMergeConnectedChatBlocks(block) ChatMessageRepository.MessagesRange( oldestMessageId = oldestId, diff --git a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatBlocksDao.kt b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatBlocksDao.kt index 9657f769339..91d4730732e 100644 --- a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatBlocksDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatBlocksDao.kt @@ -7,6 +7,7 @@ package com.nextcloud.talk.data.database.dao +import android.util.Log import androidx.room.Dao import androidx.room.Delete import androidx.room.Insert @@ -19,7 +20,7 @@ import kotlinx.coroutines.flow.Flow @Dao interface ChatBlocksDao { @Delete - fun deleteChatBlocks(blocks: List) + suspend fun deleteChatBlocks(blocks: List) @Query( """ @@ -54,12 +55,12 @@ interface ChatBlocksDao { ORDER BY newestMessageId ASC """ ) - fun getConnectedChatBlocks( + suspend fun getConnectedChatBlocks( internalConversationId: String, threadId: Long?, oldestMessageId: Long, newestMessageId: Long - ): Flow> + ): List @Query( """ @@ -74,6 +75,41 @@ interface ChatBlocksDao { @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun upsertChatBlock(chatBlock: ChatBlockEntity) + /** + * Upserts [chatBlock] and merges all chat blocks it overlaps into one covering their combined + * range, as a single atomic operation. + * + * The open-path delta fetch, long polling, the insurance request, signaling and the background + * catch-up can all update the blocks of the same conversation concurrently. Without one + * transaction around upsert, connectivity query and merge, two callers could each upsert + * their block and query connectivity before seeing the other's write, leaving overlapping + * blocks behind. + */ + @Transaction + suspend fun upsertAndMergeConnectedChatBlocks(chatBlock: ChatBlockEntity) { + upsertChatBlock(chatBlock) + + val connectedChatBlocks = getConnectedChatBlocks( + internalConversationId = chatBlock.internalConversationId, + threadId = chatBlock.threadId, + oldestMessageId = chatBlock.oldestMessageId, + newestMessageId = chatBlock.newestMessageId + ) + if (connectedChatBlocks.size > 1) { + val mergedBlock = chatBlock.copy( + oldestMessageId = connectedChatBlocks.minOf { it.oldestMessageId }, + newestMessageId = connectedChatBlocks.maxOf { it.newestMessageId }, + hasHistory = connectedChatBlocks.all { it.hasHistory } + ) + replaceConnectedChatBlocks(connectedChatBlocks, mergedBlock) + Log.d( + TAG, + "Merged ${connectedChatBlocks.size} connected chat blocks into " + + "${mergedBlock.oldestMessageId}..${mergedBlock.newestMessageId}" + ) + } + } + @Transaction suspend fun replaceConnectedChatBlocks(connectedBlocks: List, mergedBlock: ChatBlockEntity) { val newestConnectedBlock = connectedBlocks.maxByOrNull { it.newestMessageId } @@ -131,4 +167,8 @@ interface ChatBlocksDao { """ ) suspend fun getChatBlocksForConversation(internalConversationId: String): List + + companion object { + private const val TAG = "ChatBlocksDao" + } } 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 950e19bea52..65dc4524f51 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 @@ -274,7 +274,7 @@ class DummyConversationDaoImpl : ConversationsDao { } class DummyChatBlocksDaoImpl : ChatBlocksDao { - override fun deleteChatBlocks(blocks: List) { + override suspend fun deleteChatBlocks(blocks: List) { /* */ } @@ -284,12 +284,12 @@ class DummyChatBlocksDaoImpl : ChatBlocksDao { messageId: Long ): Flow> = flowOf() - override fun getConnectedChatBlocks( + override suspend fun getConnectedChatBlocks( internalConversationId: String, threadId: Long?, oldestMessageId: Long, newestMessageId: Long - ): Flow> = flowOf() + ): List = emptyList() override fun getNewestMessageIdFromChatBlocks(internalConversationId: String, threadId: Long?): Long = 0L diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt index 21fd18acfc6..1ffedb5b0a6 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncerTest.kt @@ -139,8 +139,6 @@ class ChatMessageSyncerTest { .thenReturn(42L) whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) .thenReturn(flowOf(listOf(existingBlock))) - whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) - .thenReturn(flowOf(listOf(existingBlock))) wheneverBlocking { network.pullChatMessages(any(), any(), any()) } .thenReturn(Response.success(overall(message(43), message(44)))) @@ -158,7 +156,7 @@ class ChatMessageSyncerTest { assertEquals(0, fieldMapCaptor.firstValue["markNotificationsAsRead"]) val blockCaptor = argumentCaptor() - verifyBlocking(chatBlocksDao) { upsertChatBlock(blockCaptor.capture()) } + verifyBlocking(chatBlocksDao) { upsertAndMergeConnectedChatBlocks(blockCaptor.capture()) } assertEquals(10L, blockCaptor.firstValue.oldestMessageId) assertEquals(44L, blockCaptor.firstValue.newestMessageId) } @@ -168,8 +166,6 @@ class ChatMessageSyncerTest { runTest { whenever(chatBlocksDao.getNewestMessageIdFromChatBlocks(INTERNAL_CONVERSATION_ID, null)) .thenReturn(0L) - whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) - .thenReturn(flowOf(emptyList())) wheneverBlocking { network.pullChatMessages(any(), any(), any()) } .thenReturn(Response.success(overall(message(1), message(2), message(3)))) @@ -188,7 +184,7 @@ class ChatMessageSyncerTest { assertEquals(0, fieldMapCaptor.firstValue["markNotificationsAsRead"]) val blockCaptor = argumentCaptor() - verifyBlocking(chatBlocksDao) { upsertChatBlock(blockCaptor.capture()) } + verifyBlocking(chatBlocksDao) { upsertAndMergeConnectedChatBlocks(blockCaptor.capture()) } assertEquals(1L, blockCaptor.firstValue.oldestMessageId) assertEquals(3L, blockCaptor.firstValue.newestMessageId) } @@ -201,8 +197,6 @@ class ChatMessageSyncerTest { .thenReturn(42L) whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) .thenReturn(flowOf(listOf(existingBlock))) - whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) - .thenReturn(flowOf(listOf(existingBlock))) val firstFetchStarted = CompletableDeferred() val firstFetchReleased = CompletableDeferred() @@ -236,13 +230,11 @@ class ChatMessageSyncerTest { } @Test - fun `pullAndPersistMessages merges connected chat blocks`() = + fun `pullAndPersistMessages extends the queried block and delegates the merge to the dao`() = runTest { - val connectedBlocks = listOf(block(oldest = 1, newest = 5), block(oldest = 3, newest = 44)) + val blockOfQueriedMessage = block(oldest = 3, newest = 44) whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) - .thenReturn(flowOf(listOf(connectedBlocks[1]))) - whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) - .thenReturn(flowOf(connectedBlocks)) + .thenReturn(flowOf(listOf(blockOfQueriedMessage))) wheneverBlocking { network.pullChatMessages(any(), any(), any()) } .thenReturn(Response.success(overall(message(43), message(44)))) @@ -254,10 +246,12 @@ class ChatMessageSyncerTest { ) syncer.pullAndPersistMessages(target(), fieldMap) - val mergedCaptor = argumentCaptor() - verifyBlocking(chatBlocksDao) { replaceConnectedChatBlocks(eq(connectedBlocks), mergedCaptor.capture()) } - assertEquals(1L, mergedCaptor.firstValue.oldestMessageId) - assertEquals(44L, mergedCaptor.firstValue.newestMessageId) + // merging with other overlapping blocks happens atomically inside the dao, the syncer + // only hands over the fetched range extended down to the queried block's oldest id + val blockCaptor = argumentCaptor() + verifyBlocking(chatBlocksDao) { upsertAndMergeConnectedChatBlocks(blockCaptor.capture()) } + assertEquals(3L, blockCaptor.firstValue.oldestMessageId) + assertEquals(44L, blockCaptor.firstValue.newestMessageId) } @Test @@ -280,7 +274,7 @@ class ChatMessageSyncerTest { assertFalse(outcome.persistedNewMessages) assertNull(outcome.newestPersistedMessageId) - verifyBlocking(chatBlocksDao, never()) { upsertChatBlock(any()) } + verifyBlocking(chatBlocksDao, never()) { upsertAndMergeConnectedChatBlocks(any()) } } @Test @@ -315,8 +309,6 @@ class ChatMessageSyncerTest { val existingBlock = block(oldest = 10, newest = 42) whenever(chatBlocksDao.getChatBlocksContainingMessageId(eq(INTERNAL_CONVERSATION_ID), eq(null), any())) .thenReturn(flowOf(listOf(existingBlock))) - whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) - .thenReturn(flowOf(listOf(existingBlock))) wheneverBlocking { network.pullChatMessages(any(), any(), any()) } .thenReturn( Response.success(overall(message(43), message(44))), @@ -342,8 +334,6 @@ class ChatMessageSyncerTest { val existingBlock = block(oldest = 10, newest = 42) whenever(chatBlocksDao.getChatBlocksContainingMessageId(eq(INTERNAL_CONVERSATION_ID), eq(null), any())) .thenReturn(flowOf(listOf(existingBlock))) - whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) - .thenReturn(flowOf(listOf(existingBlock))) wheneverBlocking { network.pullChatMessages(any(), any(), any()) } .thenReturn( Response.success(overall(message(43))), @@ -378,8 +368,6 @@ class ChatMessageSyncerTest { val existingBlock = block(oldest = 10, newest = 42) whenever(chatBlocksDao.getChatBlocksContainingMessageId(INTERNAL_CONVERSATION_ID, null, 42L)) .thenReturn(flowOf(listOf(existingBlock))) - whenever(chatBlocksDao.getConnectedChatBlocks(eq(INTERNAL_CONVERSATION_ID), eq(null), any(), any())) - .thenReturn(flowOf(listOf(existingBlock))) wheneverBlocking { network.pullChatMessages(any(), any(), any()) } .thenReturn(Response.success(overall(message(43), message(44))))