From b6e341bbf13a4a28890a32d4aea7e348d60f6509 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 22 Jan 2025 13:50:30 +0100 Subject: [PATCH 1/3] cancel scope when onStop is reached This is necessary especially to cancel the long polling when configuration change was made, e.g. screen was rotated. Otherwise multiple long polling requests would be running after configuration changes. Because it not possible to launch a new coroutine in a scope that was canceled, it is necessary to re-initialize the scope. Signed-off-by: Marcel Hibbe --- .../java/com/nextcloud/talk/chat/ChatActivity.kt | 10 ++++------ .../talk/chat/data/ChatMessageRepository.kt | 2 +- .../chat/data/network/OfflineFirstChatRepository.kt | 12 +++++++++--- .../nextcloud/talk/chat/viewmodels/ChatViewModel.kt | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt index 9af2aaa32..e0d32e9b7 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt @@ -680,12 +680,10 @@ class ChatActivity : val urlForChatting = ApiUtils.getUrlForChat(chatApiVersion, conversationUser?.baseUrl, roomToken) - if (adapter?.isEmpty == true) { - chatViewModel.loadMessages( - withCredentials = credentials!!, - withUrl = urlForChatting - ) - } + chatViewModel.loadMessages( + withCredentials = credentials!!, + withUrl = urlForChatting + ) } else { Log.w( TAG, 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 d42c3bff3..3f1ac075a 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 @@ -46,7 +46,7 @@ interface ChatMessageRepository : LifecycleAwareManager { fun setData(conversationModel: ConversationModel, credentials: String, urlForChatting: String) - fun loadInitialMessages(withNetworkParams: Bundle): Job + fun initScopeAndLoadInitialMessages(withNetworkParams: Bundle) /** * Loads messages from local storage. If the messages are not found, then it 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 90ce591d8..2953fab83 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 @@ -45,6 +45,7 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.retryWhen +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import java.io.IOException import javax.inject.Inject @@ -111,7 +112,7 @@ class OfflineFirstChatRepository @Inject constructor( private var newXChatLastCommonRead: Int? = null private var itIsPaused = false - private val scope = CoroutineScope(Dispatchers.IO) + private lateinit var scope: CoroutineScope lateinit var internalConversationId: String private lateinit var conversationModel: ConversationModel @@ -125,7 +126,12 @@ class OfflineFirstChatRepository @Inject constructor( internalConversationId = conversationModel.internalId } - override fun loadInitialMessages(withNetworkParams: Bundle): Job = + override fun initScopeAndLoadInitialMessages(withNetworkParams: Bundle) { + scope = CoroutineScope(Dispatchers.IO) + loadInitialMessages(withNetworkParams) + } + + private fun loadInitialMessages(withNetworkParams: Bundle): Job = scope.launch { Log.d(TAG, "---- loadInitialMessages ------------") newXChatLastCommonRead = conversationModel.lastCommonReadMessage @@ -793,7 +799,7 @@ class OfflineFirstChatRepository @Inject constructor( } override fun handleOnStop() { - // unused atm + scope.cancel() } override fun handleChatOnBackPress() { 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 bb2abef36..b92433fd2 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 @@ -395,7 +395,7 @@ class ChatViewModel @Inject constructor( val bundle = Bundle() bundle.putString(BundleKeys.KEY_CHAT_URL, withUrl) bundle.putString(BundleKeys.KEY_CREDENTIALS, withCredentials) - chatRepository.loadInitialMessages( + chatRepository.initScopeAndLoadInitialMessages( withNetworkParams = bundle ) } From 1f105d44d36e8bf30631703eef67dbd7e4f2fde6 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 22 Jan 2025 13:54:31 +0100 Subject: [PATCH 2/3] check if scope is active during long polling Because long polling may have to be informed that the scopep was cancelled, the isActive checks are added so it can't happen that messages are added when they shouldn't. I could not reproduce the scenario, anyway the checks should make sense in my opinion. Signed-off-by: Marcel Hibbe --- .../data/network/OfflineFirstChatRepository.kt | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 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 2953fab83..c8577e418 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 @@ -308,7 +308,7 @@ class OfflineFirstChatRepository @Inject constructor( var showUnreadMessagesMarker = true - while (true) { + while (isActive) { if (!networkMonitor.isOnline.value || itIsPaused) { Thread.sleep(HALF_SECOND) } else { @@ -324,11 +324,15 @@ class OfflineFirstChatRepository @Inject constructor( val weHaveMessagesFromOurself = chatMessages.any { it.actorId == currentUser.userId } showUnreadMessagesMarker = showUnreadMessagesMarker && !weHaveMessagesFromOurself - handleNewAndTempMessages( - receivedChatMessages = chatMessages, - lookIntoFuture = true, - showUnreadMessagesMarker = showUnreadMessagesMarker - ) + if (isActive) { + handleNewAndTempMessages( + receivedChatMessages = chatMessages, + lookIntoFuture = true, + showUnreadMessagesMarker = showUnreadMessagesMarker + ) + } else { + Log.d(TAG, "scope was already canceled") + } } else { Log.d(TAG, "resultsFromSync are null or empty") } From 9b6b01254ad3460dcc8fa5a9ff8131517497bb76 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 22 Jan 2025 13:59:03 +0100 Subject: [PATCH 3/3] remove scope cancelation for handleChatOnBackPress (replace with lifecyle event) handleOnStop will handle this (and more scenarios than just backpress) Signed-off-by: Marcel Hibbe --- app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt | 1 - .../com/nextcloud/talk/chat/data/ChatMessageRepository.kt | 5 ----- .../talk/chat/data/network/OfflineFirstChatRepository.kt | 4 ---- .../java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt | 4 ---- 4 files changed, 14 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt index e0d32e9b7..75f7114f1 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt @@ -348,7 +348,6 @@ class ChatActivity : private val onBackPressedCallback = object : OnBackPressedCallback(true) { override fun handleOnBackPressed() { - chatViewModel.handleChatOnBackPress() if (currentlyPlayedVoiceMessage != null) { stopMediaPlayer(currentlyPlayedVoiceMessage!!) } 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 3f1ac075a..2cc6979ff 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 @@ -74,11 +74,6 @@ interface ChatMessageRepository : LifecycleAwareManager { */ suspend fun getMessage(messageId: Long, bundle: Bundle): Flow - /** - * Destroys unused resources. - */ - fun handleChatOnBackPress() - @Suppress("LongParameterList") suspend fun sendChatMessage( credentials: String, 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 c8577e418..3e731c40d 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 @@ -806,10 +806,6 @@ class OfflineFirstChatRepository @Inject constructor( scope.cancel() } - override fun handleChatOnBackPress() { - scope.cancel() - } - @Suppress("LongParameterList") override suspend fun sendChatMessage( credentials: String, 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 b92433fd2..a4a8ad538 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 @@ -647,10 +647,6 @@ class ChatViewModel @Inject constructor( _getCapabilitiesViewState.value = GetCapabilitiesStartState } - fun handleChatOnBackPress() { - chatRepository.handleChatOnBackPress() - } - fun getMessageById(url: String, conversationModel: ConversationModel, messageId: Long): Flow = flow { val bundle = Bundle()