fix to leave room before loading new one

-> add callback methods to ConductorRemapping to execute after chat room was left. Whenever there is a ChatController on top, it's room is now left, before replacing the controller or pushing another one on top.

this avoids problems where entering a chat before the old one was left led to sessionId="0" for the new chat.

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>

app/src/main/java/com/nextcloud/talk/activities/MainActivity.kt

@@ -50,13 +50,13 @@ import com.nextcloud.talk.databinding.ActivityMainBinding
 import com.nextcloud.talk.models.json.conversations.RoomOverall
 import com.nextcloud.talk.users.UserManager
 import com.nextcloud.talk.utils.ApiUtils
-import com.nextcloud.talk.utils.ConductorRemapping.remapChatController
 import com.nextcloud.talk.utils.SecurityUtils
 import com.nextcloud.talk.utils.bundle.BundleKeys
 import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ACTIVE_CONVERSATION
 import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_ID
 import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN
 import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_USER_ENTITY
+import com.nextcloud.talk.utils.remapchat.ConductorRemapping.remapChatController
 import io.reactivex.Observer
 import io.reactivex.SingleObserver
 import io.reactivex.android.schedulers.AndroidSchedulers

app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt

@@ -96,7 +96,6 @@ import coil.load
 import coil.request.ImageRequest
 import coil.target.Target
 import coil.transform.CircleCropTransformation
-import com.bluelinelabs.conductor.Router
 import com.bluelinelabs.conductor.RouterTransaction
 import com.bluelinelabs.conductor.changehandler.HorizontalChangeHandler
 import com.google.android.flexbox.FlexboxLayout
@@ -162,8 +161,6 @@ import com.nextcloud.talk.ui.dialog.ShowReactionsDialog
 import com.nextcloud.talk.ui.recyclerview.MessageSwipeActions
 import com.nextcloud.talk.ui.recyclerview.MessageSwipeCallback
 import com.nextcloud.talk.utils.ApiUtils
-import com.nextcloud.talk.utils.ConductorRemapping
-import com.nextcloud.talk.utils.ConductorRemapping.remapChatController
 import com.nextcloud.talk.utils.ContactUtils
 import com.nextcloud.talk.utils.DateConstants
 import com.nextcloud.talk.utils.DateUtils
@@ -182,6 +179,8 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN
 import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_USER_ENTITY
 import com.nextcloud.talk.utils.database.user.CapabilitiesUtilNew
 import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil
+import com.nextcloud.talk.utils.remapchat.ConductorRemapping
+import com.nextcloud.talk.utils.remapchat.RemapChatModel
 import com.nextcloud.talk.utils.rx.DisposableSet
 import com.nextcloud.talk.utils.singletons.ApplicationWideCurrentRoomHolder
 import com.nextcloud.talk.utils.text.Spans
@@ -1850,18 +1849,11 @@ class ChatController(args: Bundle) :
 
         if (conversationUser != null && isActivityNotChangingConfigurations() && isNotInCall()) {
             ApplicationWideCurrentRoomHolder.getInstance().clear()
-            // sessionId is sometimes 0 here! this causes that leaveRoom is not executed and callbacks continue to
-            // live which causes bugs!!!
             if (inConversation && validSessionId()) {
-                leaveRoom()
+                leaveRoom(null, null)
             } else {
-                Log.e(TAG, "not leaving room (inConversation is false and/or validSessionId is false)")
-                if (BuildConfig.DEBUG) {
-                    Toast.makeText(
-                        context, "not leaving room (inConversation is false and/or validSessionId is false)",
-                        Toast.LENGTH_LONG
-                    ).show()
-                }
+                Log.d(TAG, "not leaving room (inConversation is false and/or validSessionId is false)")
+                // room might have already been left...
             }
         } else {
             Log.e(TAG, "not leaving room...")
@@ -1908,7 +1900,7 @@ class ChatController(args: Bundle) :
         currentlyPlayedVoiceMessage?.let { stopMediaPlayer(it) }
 
         adapter = null
-        inConversation = false // move to onDetach??
+        inConversation = false
         Log.d(TAG, "inConversation was set to false!")
     }
 
@@ -2005,33 +1997,9 @@ class ChatController(args: Bundle) :
         }
     }
 
-    private fun leaveRoom() {
-        leaveRoom(
-            null,
-            null,
-            null,
-            null,
-            null,
-            null,
-        )
-    }
-
-    private fun leaveRoom(
-        router: Router?,
-        internalUserId: Long?,
-        roomTokenOrId: String?,
-        bundle: Bundle?,
-        replaceTop: Boolean?,
-        remapChatController:
-            (
-                (
-                    router: Router,
-                    internalUserId: Long,
-                    roomTokenOrId: String,
-                    bundle: Bundle,
-                    replaceTop: Boolean
-                ) -> Unit
-            )?
+    fun leaveRoom(
+        remapChatModel: RemapChatModel?,
+        funToCallWhenLeaveSuccessful: ((RemapChatModel) -> Unit)?
     ) {
         logConversationInfos("leaveRoom")
 
@@ -2086,15 +2054,9 @@ class ChatController(args: Bundle) :
 
                     currentConversation?.sessionId = "0"
 
-                    if (remapChatController != null) {
-                        Log.d(TAG, "remapChatController was set and is now executed after room was already left")
-                        remapChatController(
-                            router!!,
-                            internalUserId!!,
-                            roomTokenOrId!!,
-                            bundle!!,
-                            replaceTop!!,
-                        )
+                    if (remapChatModel != null && funToCallWhenLeaveSuccessful != null) {
+                        Log.d(TAG, "a callback action was set and is now executed because room was left successfully")
+                        funToCallWhenLeaveSuccessful(remapChatModel)
                     } else {
                         Log.d(TAG, "remapChatController was not set")
                     }
@@ -3069,7 +3031,8 @@ class ChatController(args: Bundle) :
                                     KEY_ACTIVE_CONVERSATION,
                                     Parcels.wrap(roomOverall.ocs!!.data!!)
                                 )
-                                remapChatController(
+
+                                ConductorRemapping.remapChatController(
                                     router,
                                     conversationUser!!.id!!,
                                     roomOverall.ocs!!.data!!.token!!,
@@ -3394,13 +3357,12 @@ class ChatController(args: Bundle) :
                             )
                             conversationIntent.putExtras(bundle)
 
-                            leaveRoom(
+                            ConductorRemapping.remapChatController(
                                 router,
                                 conversationUser.id!!,
                                 roomOverall.ocs!!.data!!.token!!,
                                 bundle,
-                                false,
-                                ConductorRemapping::remapChatController
+                                true
                             )
                         } else {
                             conversationIntent.putExtras(bundle)

app/src/main/java/com/nextcloud/talk/controllers/ContactsController.kt

@@ -66,9 +66,9 @@ import com.nextcloud.talk.models.json.participants.Participant
 import com.nextcloud.talk.ui.dialog.ContactsBottomDialog
 import com.nextcloud.talk.users.UserManager
 import com.nextcloud.talk.utils.ApiUtils
-import com.nextcloud.talk.utils.ConductorRemapping
 import com.nextcloud.talk.utils.bundle.BundleKeys
 import com.nextcloud.talk.utils.database.user.CapabilitiesUtilNew
+import com.nextcloud.talk.utils.remapchat.ConductorRemapping
 import eu.davidea.flexibleadapter.FlexibleAdapter
 import eu.davidea.flexibleadapter.SelectableAdapter
 import eu.davidea.flexibleadapter.common.SmoothScrollLinearLayoutManager
@@ -285,8 +285,11 @@ class ContactsController(args: Bundle) :
                                     Parcels.wrap(roomOverall.ocs!!.data!!)
                                 )
                                 ConductorRemapping.remapChatController(
-                                    router, currentUser!!.id!!,
-                                    roomOverall.ocs!!.data!!.token!!, bundle, true
+                                    router,
+                                    currentUser!!.id!!,
+                                    roomOverall.ocs!!.data!!.token!!,
+                                    bundle,
+                                    true
                                 )
                             }
 
@@ -738,9 +741,11 @@ class ContactsController(args: Bundle) :
     @Subscribe(threadMode = ThreadMode.MAIN)
     fun onMessageEvent(openConversationEvent: OpenConversationEvent) {
         ConductorRemapping.remapChatController(
-            router, currentUser!!.id!!,
+            router,
+            currentUser!!.id!!,
             openConversationEvent.conversation!!.token!!,
-            openConversationEvent.bundle!!, true
+            openConversationEvent.bundle!!,
+            true
         )
         contactsBottomDialog?.dismiss()
     }

app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.kt

@@ -99,7 +99,6 @@ import com.nextcloud.talk.ui.dialog.ConversationsListBottomDialog
 import com.nextcloud.talk.users.UserManager
 import com.nextcloud.talk.utils.ApiUtils
 import com.nextcloud.talk.utils.ClosedInterfaceImpl
-import com.nextcloud.talk.utils.ConductorRemapping.remapChatController
 import com.nextcloud.talk.utils.FileUtils
 import com.nextcloud.talk.utils.Mimetype
 import com.nextcloud.talk.utils.ParticipantPermissions
@@ -119,6 +118,7 @@ import com.nextcloud.talk.utils.database.user.CapabilitiesUtilNew.hasSpreedFeatu
 import com.nextcloud.talk.utils.database.user.CapabilitiesUtilNew.isServerEOL
 import com.nextcloud.talk.utils.database.user.CapabilitiesUtilNew.isUnifiedSearchAvailable
 import com.nextcloud.talk.utils.database.user.CapabilitiesUtilNew.isUserStatusAvailable
+import com.nextcloud.talk.utils.remapchat.ConductorRemapping.remapChatController
 import com.nextcloud.talk.utils.rx.SearchViewObservable.Companion.observeSearchView
 import eu.davidea.flexibleadapter.FlexibleAdapter
 import eu.davidea.flexibleadapter.common.SmoothScrollLinearLayoutManager

app/src/main/java/com/nextcloud/talk/ui/bottom/sheet/ProfileBottomSheet.kt

@@ -41,8 +41,8 @@ import com.nextcloud.talk.ui.bottom.sheet.ProfileBottomSheet.AllowedAppIds.EMAIL
 import com.nextcloud.talk.ui.bottom.sheet.ProfileBottomSheet.AllowedAppIds.PROFILE
 import com.nextcloud.talk.ui.bottom.sheet.ProfileBottomSheet.AllowedAppIds.SPREED
 import com.nextcloud.talk.utils.ApiUtils
-import com.nextcloud.talk.utils.ConductorRemapping
 import com.nextcloud.talk.utils.bundle.BundleKeys
+import com.nextcloud.talk.utils.remapchat.ConductorRemapping
 import io.reactivex.Observer
 import io.reactivex.android.schedulers.AndroidSchedulers
 import io.reactivex.disposables.Disposable

app/src/main/java/com/nextcloud/talk/utils/ConductorRemapping.kt → app/src/main/java/com/nextcloud/talk/utils/remapchat/ConductorRemapping.kt

@@ -18,7 +18,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-package com.nextcloud.talk.utils
+package com.nextcloud.talk.utils.remapchat
 
 import android.os.Bundle
 import android.util.Log
@@ -62,34 +62,43 @@ object ConductorRemapping {
             } else {
                 HorizontalChangeHandler()
             }
-            if (!replaceTop) {
-                if (!router.hasRootController()) {
-                    Log.d(TAG, "router has no RootController. creating backstack with ConversationsListController")
-                    val newBackstack = listOf(
-                        RouterTransaction.with(ConversationsListController(Bundle()))
-                            .pushChangeHandler(HorizontalChangeHandler())
-                            .popChangeHandler(HorizontalChangeHandler()),
-                        RouterTransaction.with(ChatController(bundle))
-                            .pushChangeHandler(HorizontalChangeHandler())
-                            .popChangeHandler(HorizontalChangeHandler()).tag(chatControllerTag)
-                    )
-                    router.setBackstack(newBackstack, SimpleSwapChangeHandler())
+
+            if (router.hasRootController()) {
+                val backstack = router.backstack
+                val topController = backstack[router.backstackSize - 1].controller
+
+                val remapChatModel = RemapChatModel(
+                    router,
+                    pushChangeHandler,
+                    chatControllerTag,
+                    bundle
+                )
+
+                if (topController is ChatController) {
+                    if (replaceTop) {
+                        topController.leaveRoom(remapChatModel, this::replaceTopController)
+                    } else {
+                        topController.leaveRoom(remapChatModel, this::pushController)
+                    }
                 } else {
-                    Log.d(TAG, "router has RootController. pushing ChatController")
-                    router.pushController(
-                        RouterTransaction.with(ChatController(bundle))
-                            .pushChangeHandler(pushChangeHandler)
-                            .popChangeHandler(HorizontalChangeHandler()).tag(chatControllerTag)
-                    )
+                    if (replaceTop) {
+                        replaceTopController(remapChatModel)
+                    } else {
+                        pushController(remapChatModel)
+                    }
                 }
             } else {
-                Log.d(TAG, "ChatController replace topController")
-
-                router.replaceTopController(
+                Log.d(TAG, "router has no RootController. creating backstack with ConversationsListController")
+                val newBackstack = listOf(
+                    RouterTransaction.with(ConversationsListController(Bundle()))
+                        .pushChangeHandler(HorizontalChangeHandler())
+                        .popChangeHandler(HorizontalChangeHandler()),
                     RouterTransaction.with(ChatController(bundle))
-                        .pushChangeHandler(pushChangeHandler)
-                        .popChangeHandler(HorizontalChangeHandler()).tag(chatControllerTag)
+                        .pushChangeHandler(HorizontalChangeHandler())
+                        .popChangeHandler(HorizontalChangeHandler())
+                        .tag(chatControllerTag)
                 )
+                router.setBackstack(newBackstack, SimpleSwapChangeHandler())
             }
         }
 
@@ -98,6 +107,26 @@ object ConductorRemapping {
         }
     }
 
+    fun pushController(remapChatModel: RemapChatModel) {
+        Log.d(TAG, "pushController")
+        remapChatModel.router.pushController(
+            RouterTransaction.with(ChatController(remapChatModel.bundle))
+                .pushChangeHandler(remapChatModel.controllerChangeHandler)
+                .popChangeHandler(HorizontalChangeHandler())
+                .tag(remapChatModel.chatControllerTag)
+        )
+    }
+
+    private fun replaceTopController(remapChatModel: RemapChatModel) {
+        Log.d(TAG, "replaceTopController")
+        remapChatModel.router.replaceTopController(
+            RouterTransaction.with(ChatController(remapChatModel.bundle))
+                .pushChangeHandler(remapChatModel.controllerChangeHandler)
+                .popChangeHandler(HorizontalChangeHandler())
+                .tag(remapChatModel.chatControllerTag)
+        )
+    }
+
     private fun moveControllerToTop(router: Router, controllerTag: String) {
         Log.d(TAG, "moving $controllerTag to top...")
         val backstack = router.backstack

app/src/main/java/com/nextcloud/talk/utils/remapchat/RemapChatModel.kt

@@ -0,0 +1,12 @@
+package com.nextcloud.talk.utils.remapchat
+
+import android.os.Bundle
+import com.bluelinelabs.conductor.ControllerChangeHandler
+import com.bluelinelabs.conductor.Router
+
+data class RemapChatModel(
+    val router: Router,
+    val controllerChangeHandler: ControllerChangeHandler,
+    val chatControllerTag: String,
+    val bundle: Bundle
+)