Merge pull request #4641 from nextcloud/bugfix/4605/fixDuplicateMessages

Bugfix/4605/fix duplicate messages

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

@@ -347,7 +347,6 @@ class ChatActivity :
 
     private val onBackPressedCallback = object : OnBackPressedCallback(true) {
         override fun handleOnBackPressed() {
-            chatViewModel.handleChatOnBackPress()
             if (currentlyPlayedVoiceMessage != null) {
                 stopMediaPlayer(currentlyPlayedVoiceMessage!!)
             }
@@ -674,12 +673,10 @@ class ChatActivity :
                         val urlForChatting =
                             ApiUtils.getUrlForChat(chatApiVersion, conversationUser?.baseUrl, roomToken)
 
-                        if (adapter?.isEmpty == true) {
+                        chatViewModel.loadMessages(
-                            chatViewModel.loadMessages(
+                            withCredentials = credentials!!,
-                                withCredentials = credentials!!,
+                            withUrl = urlForChatting
-                                withUrl = urlForChatting
+                        )
-                            )
-                        }
                     } else {
                         Log.w(
                             TAG,

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
@@ -74,11 +74,6 @@ interface ChatMessageRepository : LifecycleAwareManager {
      */
     suspend fun getMessage(messageId: Long, bundle: Bundle): Flow<ChatMessage>
 
-    /**
-     * Destroys unused resources.
-     */
-    fun handleChatOnBackPress()
-
     @Suppress("LongParameterList")
     suspend fun sendChatMessage(
         credentials: String,

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
@@ -302,7 +308,7 @@ class OfflineFirstChatRepository @Inject constructor(
 
             var showUnreadMessagesMarker = true
 
-            while (true) {
+            while (isActive) {
                 if (!networkMonitor.isOnline.value || itIsPaused) {
                     Thread.sleep(HALF_SECOND)
                 } else {
@@ -318,11 +324,15 @@ class OfflineFirstChatRepository @Inject constructor(
                         val weHaveMessagesFromOurself = chatMessages.any { it.actorId == currentUser.userId }
                         showUnreadMessagesMarker = showUnreadMessagesMarker && !weHaveMessagesFromOurself
 
-                        handleNewAndTempMessages(
+                        if (isActive) {
-                            receivedChatMessages = chatMessages,
+                            handleNewAndTempMessages(
-                            lookIntoFuture = true,
+                                receivedChatMessages = chatMessages,
-                            showUnreadMessagesMarker = showUnreadMessagesMarker
+                                lookIntoFuture = true,
-                        )
+                                showUnreadMessagesMarker = showUnreadMessagesMarker
+                            )
+                        } else {
+                            Log.d(TAG, "scope was already canceled")
+                        }
                     } else {
                         Log.d(TAG, "resultsFromSync are null or empty")
                     }
@@ -793,10 +803,6 @@ class OfflineFirstChatRepository @Inject constructor(
     }
 
     override fun handleOnStop() {
-        // unused atm
-    }
-
-    override fun handleChatOnBackPress() {
         scope.cancel()
     }
 

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
         )
     }
@@ -647,10 +647,6 @@ class ChatViewModel @Inject constructor(
         _getCapabilitiesViewState.value = GetCapabilitiesStartState
     }
 
-    fun handleChatOnBackPress() {
-        chatRepository.handleChatOnBackPress()
-    }
-
     fun getMessageById(url: String, conversationModel: ConversationModel, messageId: Long): Flow<ChatMessage> =
         flow {
             val bundle = Bundle()