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 5a9b74eeb4f..71db2106704 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 @@ -714,9 +714,15 @@ class ChatMessageSyncer @Inject constructor( * [MAX_PULL_ATTEMPTS] times with a linearly growing delay in between. The requested limit is * kept across retries: a background catch-up that succeeds on a later attempt must still * create a full chat block, and backing off in time rather than in payload size also keeps a - * struggling or rate-limiting server from being hammered. + * struggling or rate-limiting server from being hammered. A failed request is not retried + * once [canRetry] returns false, e.g. a long poll must not restart while the chat is in the + * background. */ - fun pullMessagesFlow(target: SyncTarget, fieldMap: HashMap): Flow = + fun pullMessagesFlow( + target: SyncTarget, + fieldMap: HashMap, + canRetry: () -> Boolean = { true } + ): Flow = flow { var attempts = 1 @@ -741,8 +747,17 @@ class ChatMessageSyncer @Inject constructor( onFailure = { e -> Log.e(TAG, "Attempt $attempts failed", e) attempts++ + if (!canRetry()) { + emit(ChatPullResult.Error(IllegalStateException("Retry cancelled"))) + return@flow + } if (attempts < MAX_PULL_ATTEMPTS) { delay(PULL_RETRY_DELAY_MILLIS * (attempts - 1)) + // the chat may have been paused while backing off + if (!canRetry()) { + emit(ChatPullResult.Error(IllegalStateException("Retry cancelled"))) + return@flow + } } } ) @@ -758,7 +773,8 @@ class ChatMessageSyncer @Inject constructor( suspend fun pullAndPersistMessages( target: SyncTarget, fieldMap: HashMap, - events: Events = NO_EVENTS + events: Events = NO_EVENTS, + canRetry: () -> Boolean = { true } ): SyncOutcome { val isLongPoll = (fieldMap["timeout"] ?: 0) > 0 if (!isLongPoll) events.onLoadingChanged(true) @@ -770,7 +786,7 @@ class ChatMessageSyncer @Inject constructor( val queriedMessageId = fieldMap["lastKnownMessageId"] val lookIntoFuture = fieldMap["lookIntoFuture"] == 1 - return when (val result = pullMessagesFlow(target, fieldMap).first()) { + return when (val result = pullMessagesFlow(target, fieldMap, canRetry).first()) { is ChatPullResult.Success -> handleSuccessfulPull(target, result, queriedMessageId, lookIntoFuture, events) 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 180f3cb17eb..1d2eb4902b4 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 @@ -318,7 +318,7 @@ class OfflineFirstChatRepository @Inject constructor( // sync database with server // (This is a long blocking call because long polling (lookIntoFuture and timeout) is set) Log.d(TAG, "Starting online request for long polling") - val outcome = syncer.pullAndPersistMessages(syncTarget, fieldMap, syncEvents) + val outcome = syncer.pullAndPersistMessages(syncTarget, fieldMap, syncEvents) { !itIsPaused } // A failed request returns right away, e.g. with rejected credentials, which the server counts as a // failed login. Without waiting, the next requests would follow at once until the server throttles. 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 7df743cdcce..e2f694d0756 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 @@ -9,6 +9,7 @@ package com.nextcloud.talk.chat.data.network import android.database.sqlite.SQLiteConstraintException import com.nextcloud.talk.chat.data.model.ChatMessage +import com.nextcloud.talk.chat.domain.ChatPullResult import com.nextcloud.talk.conversationlist.data.network.ConversationListUpdater import com.nextcloud.talk.data.database.dao.ChatBlocksDao import com.nextcloud.talk.data.database.dao.ChatMessagesDao @@ -25,6 +26,7 @@ import com.nextcloud.talk.utils.CapabilitiesUtil import java.io.IOException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest @@ -235,6 +237,32 @@ class ChatMessageSyncerTest { assertEquals(REQUESTED_LIMIT, fieldMapCaptor.lastValue["limit"]) } + @Test + fun `pullMessagesFlow does not retry a failed request once retrying is not allowed`() = + runTest { + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .doSuspendableAnswer { throw IOException("connection reset") } + + val result = syncer.pullMessagesFlow(target(), HashMap(), canRetry = { false }).first() + + assertTrue(result is ChatPullResult.Error) + verifyBlocking(network, times(1)) { pullChatMessages(any(), any(), any()) } + } + + @Test + fun `pullMessagesFlow does not send the retry when the chat is paused during the backoff`() = + runTest { + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .doSuspendableAnswer { throw IOException("connection reset") } + // allowed right after the failure, paused by the time the backoff delay has elapsed + val answers = mutableListOf(true, false) + + val result = syncer.pullMessagesFlow(target(), HashMap(), canRetry = { answers.removeFirst() }).first() + + assertTrue(result is ChatPullResult.Error) + verifyBlocking(network, times(1)) { pullChatMessages(any(), any(), any()) } + } + @Test fun `catchUpRoom marks the sync as failed when the server request errors`() = runTest {