Space Switching Refactoring and Unit Tests

changelog.d/6598.misc

@@ -0,0 +1 @@
+Refactors SpaceStateHandler (previously AppStateHandler) and adds unit tests for it

vector/src/main/java/im/vector/app/SpaceStateHandler.kt

@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app
+
+import androidx.lifecycle.DefaultLifecycleObserver
+import arrow.core.Option
+import kotlinx.coroutines.flow.Flow
+import org.matrix.android.sdk.api.session.Session
+import org.matrix.android.sdk.api.session.room.model.RoomSummary
+
+/**
+ * Gets info about the current space the user has navigated to, any space backstack they may have
+ * and handles switching to different spaces
+ */
+interface SpaceStateHandler : DefaultLifecycleObserver {
+
+    /**
+     * Gets the current space the current user has navigated to
+     *
+     * @return null if the user is not in
+     */
+    fun getCurrentSpace(): RoomSummary?
+
+    /**
+     * Sets the new space the current user is navigating to
+     *
+     * @param spaceId the id of the space being navigated to
+     * @param session the current active session
+     * @param persistNow if true, the current space will immediately be persisted in shared prefs
+     * @param isForwardNavigation whether this navigation is a forward action to properly handle backstack
+     */
+    fun setCurrentSpace(
+            spaceId: String?,
+            session: Session? = null,
+            persistNow: Boolean = false,
+            isForwardNavigation: Boolean = true,
+    )
+
+    /**
+     * Gets the current backstack of spaces (via their id)
+     *
+     * null may be an entry in the ArrayDeque to indicate the root space (All Chats)
+     */
+    fun getSpaceBackstack(): ArrayDeque<String?>
+
+    /**
+     * Gets a flow of the selected space for clients to react immediately to space changes
+     */
+    fun getSelectedSpaceFlow(): Flow<Option<RoomSummary>>
+
+    /**
+     * Gets the id of the active space, or null if there is none
+     */
+    fun getSafeActiveSpaceId(): String?
+}

vector/src/main/java/im/vector/app/AppStateHandler.kt → vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt

@@ -16,7 +16,6 @@
 
 package im.vector.app
 
-import androidx.lifecycle.DefaultLifecycleObserver
 import androidx.lifecycle.LifecycleOwner
 import arrow.core.Option
 import im.vector.app.core.di.ActiveSessionHolder
@@ -46,54 +45,60 @@ import javax.inject.Singleton
 
 /**
  * This class handles the global app state.
- * It requires to be added to ProcessLifecycleOwner.get().lifecycle
+ * It is required that this class is added as an observer to ProcessLifecycleOwner.get().lifecycle in [VectorApplication]
  */
-// TODO Keep this class for now, will maybe be used fro Space
 @Singleton
-class AppStateHandler @Inject constructor(
+class SpaceStateHandlerImpl @Inject constructor(
         private val sessionDataSource: ActiveSessionDataSource,
         private val uiStateRepository: UiStateRepository,
         private val activeSessionHolder: ActiveSessionHolder,
         private val analyticsTracker: AnalyticsTracker
-) : DefaultLifecycleObserver {
+) : SpaceStateHandler {
 
     private val coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main)
     private val selectedSpaceDataSource = BehaviorDataSource<Option<RoomSummary>>(Option.empty())
-
-    val selectedSpaceFlow = selectedSpaceDataSource.stream()
-
+    private val selectedSpaceFlow = selectedSpaceDataSource.stream()
     private val spaceBackstack = ArrayDeque<String?>()
 
-    fun getCurrentSpace(): RoomSummary? {
+    override fun getCurrentSpace(): RoomSummary? {
         return selectedSpaceDataSource.currentValue?.orNull()?.let { spaceSummary ->
             activeSessionHolder.getSafeActiveSession()?.roomService()?.getRoomSummary(spaceSummary.roomId)
         }
     }
 
-    fun setCurrentSpace(spaceId: String?, session: Session? = null, persistNow: Boolean = false, isForwardNavigation: Boolean = true) {
+    override fun setCurrentSpace(
+            spaceId: String?,
+            session: Session?,
+            persistNow: Boolean,
+            isForwardNavigation: Boolean,
+    ) {
+        val activeSession = session ?: activeSessionHolder.getSafeActiveSession() ?: return
         val currentSpace = selectedSpaceDataSource.currentValue?.orNull()
-        val uSession = session ?: activeSessionHolder.getSafeActiveSession() ?: return
-        if (currentSpace != null && spaceId == currentSpace.roomId) return
-        val spaceSum = spaceId?.let { uSession.getRoomSummary(spaceId) }
+        val spaceSummary = spaceId?.let { activeSession.getRoomSummary(spaceId) }
+        val sameSpaceSelected = currentSpace != null && spaceId == currentSpace.roomId
+
+        if (sameSpaceSelected) {
+            return
+        }
 
         if (isForwardNavigation) {
             spaceBackstack.addLast(currentSpace?.roomId)
         }
 
         if (persistNow) {
-            uiStateRepository.storeSelectedSpace(spaceSum?.roomId, uSession.sessionId)
+            uiStateRepository.storeSelectedSpace(spaceSummary?.roomId, activeSession.sessionId)
         }
 
-        if (spaceSum == null) {
+        if (spaceSummary == null) {
             selectedSpaceDataSource.post(Option.empty())
         } else {
-            selectedSpaceDataSource.post(Option.just(spaceSum))
+            selectedSpaceDataSource.post(Option.just(spaceSummary))
         }
 
         if (spaceId != null) {
-            uSession.coroutineScope.launch(Dispatchers.IO) {
+            activeSession.coroutineScope.launch(Dispatchers.IO) {
                 tryOrNull {
-                    uSession.getRoom(spaceId)?.membershipService()?.loadRoomMembersIfNeeded()
+                    activeSession.getRoom(spaceId)?.membershipService()?.loadRoomMembersIfNeeded()
                 }
             }
         }
@@ -122,9 +127,11 @@ class AppStateHandler @Inject constructor(
                 }.launchIn(session.coroutineScope)
     }
 
-    fun getSpaceBackstack() = spaceBackstack
+    override fun getSpaceBackstack() = spaceBackstack
+
+    override fun getSelectedSpaceFlow() = selectedSpaceFlow
 
-    fun safeActiveSpaceId(): String? {
+    override fun getSafeActiveSpaceId(): String? {
         return selectedSpaceDataSource.currentValue?.orNull()?.roomId
     }
 

vector/src/main/java/im/vector/app/VectorApplication.kt

@@ -88,7 +88,7 @@ class VectorApplication :
     @Inject lateinit var vectorPreferences: VectorPreferences
     @Inject lateinit var versionProvider: VersionProvider
     @Inject lateinit var notificationUtils: NotificationUtils
-    @Inject lateinit var appStateHandler: AppStateHandler
+    @Inject lateinit var spaceStateHandler: SpaceStateHandler
     @Inject lateinit var popupAlertManager: PopupAlertManager
     @Inject lateinit var pinLocker: PinLocker
     @Inject lateinit var callManager: WebRtcCallManager
@@ -177,7 +177,7 @@ class VectorApplication :
                 fcmHelper.onEnterBackground(activeSessionHolder)
             }
         })
-        ProcessLifecycleOwner.get().lifecycle.addObserver(appStateHandler)
+        ProcessLifecycleOwner.get().lifecycle.addObserver(spaceStateHandler)
         ProcessLifecycleOwner.get().lifecycle.addObserver(pinLocker)
         ProcessLifecycleOwner.get().lifecycle.addObserver(callManager)
         // This should be done as early as possible

vector/src/main/java/im/vector/app/core/di/SingletonModule.kt

@@ -31,6 +31,8 @@ import dagger.hilt.components.SingletonComponent
 import im.vector.app.BuildConfig
 import im.vector.app.EmojiCompatWrapper
 import im.vector.app.EmojiSpanify
+import im.vector.app.SpaceStateHandler
+import im.vector.app.SpaceStateHandlerImpl
 import im.vector.app.config.analyticsConfig
 import im.vector.app.core.dispatchers.CoroutineDispatchers
 import im.vector.app.core.error.DefaultErrorFormatter
@@ -108,6 +110,9 @@ abstract class VectorBindModule {
 
     @Binds
     abstract fun bindSystemSettingsProvide(provider: AndroidSystemSettingsProvider): SystemSettingsProvider
+
+    @Binds
+    abstract fun bindSpaceStateHandler(spaceStateHandlerImpl: SpaceStateHandlerImpl): SpaceStateHandler
 }
 
 @InstallIn(SingletonComponent::class)

vector/src/main/java/im/vector/app/features/home/HomeActivity.kt

@@ -35,8 +35,8 @@ import com.airbnb.mvrx.Mavericks
 import com.airbnb.mvrx.viewModel
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import dagger.hilt.android.AndroidEntryPoint
-import im.vector.app.AppStateHandler
 import im.vector.app.R
+import im.vector.app.SpaceStateHandler
 import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.core.extensions.hideKeyboard
 import im.vector.app.core.extensions.registerStartForActivityResult
@@ -130,7 +130,7 @@ class HomeActivity :
     @Inject lateinit var permalinkHandler: PermalinkHandler
     @Inject lateinit var avatarRenderer: AvatarRenderer
     @Inject lateinit var initSyncStepFormatter: InitSyncStepFormatter
-    @Inject lateinit var appStateHandler: AppStateHandler
+    @Inject lateinit var spaceStateHandler: SpaceStateHandler
     @Inject lateinit var unifiedPushHelper: UnifiedPushHelper
     @Inject lateinit var fcmHelper: FcmHelper
     @Inject lateinit var nightlyProxy: NightlyProxy

vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt

@@ -28,8 +28,8 @@ import com.airbnb.mvrx.activityViewModel
 import com.airbnb.mvrx.fragmentViewModel
 import com.airbnb.mvrx.withState
 import com.google.android.material.badge.BadgeDrawable
-import im.vector.app.AppStateHandler
 import im.vector.app.R
+import im.vector.app.SpaceStateHandler
 import im.vector.app.core.extensions.commitTransaction
 import im.vector.app.core.extensions.toMvRxBundle
 import im.vector.app.core.platform.OnBackPressed
@@ -68,7 +68,7 @@ class HomeDetailFragment @Inject constructor(
         private val alertManager: PopupAlertManager,
         private val callManager: WebRtcCallManager,
         private val vectorPreferences: VectorPreferences,
-        private val appStateHandler: AppStateHandler,
+        private val spaceStateHandler: SpaceStateHandler,
         private val vectorFeatures: VectorFeatures,
 ) : VectorBaseFragment<FragmentHomeDetailBinding>(),
         KeysBackupBanner.Delegate,
@@ -186,13 +186,13 @@ class HomeDetailFragment @Inject constructor(
     }
 
     private fun navigateBack() {
-        val previousSpaceId = appStateHandler.getSpaceBackstack().removeLastOrNull()
-        val parentSpaceId = appStateHandler.getCurrentSpace()?.flattenParentIds?.lastOrNull()
+        val previousSpaceId = spaceStateHandler.getSpaceBackstack().removeLastOrNull()
+        val parentSpaceId = spaceStateHandler.getCurrentSpace()?.flattenParentIds?.lastOrNull()
         setCurrentSpace(previousSpaceId ?: parentSpaceId)
     }
 
     private fun setCurrentSpace(spaceId: String?) {
-        appStateHandler.setCurrentSpace(spaceId, isForwardNavigation = false)
+        spaceStateHandler.setCurrentSpace(spaceId, isForwardNavigation = false)
         sharedActionViewModel.post(HomeActivitySharedAction.OnCloseSpace)
     }
 
@@ -215,7 +215,7 @@ class HomeDetailFragment @Inject constructor(
     }
 
     private fun refreshSpaceState() {
-        appStateHandler.getCurrentSpace()?.let {
+        spaceStateHandler.getCurrentSpace()?.let {
             onSpaceChange(it)
         }
     }
@@ -473,7 +473,7 @@ class HomeDetailFragment @Inject constructor(
         return this
     }
 
-    override fun onBackPressed(toolbarButton: Boolean) = if (appStateHandler.getCurrentSpace() != null) {
+    override fun onBackPressed(toolbarButton: Boolean) = if (spaceStateHandler.getCurrentSpace() != null) {
         navigateBack()
         true
     } else {

vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt

@@ -22,7 +22,7 @@ import com.airbnb.mvrx.ViewModelContext
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
-import im.vector.app.AppStateHandler
+import im.vector.app.SpaceStateHandler
 import im.vector.app.core.di.MavericksAssistedViewModelFactory
 import im.vector.app.core.di.hiltMavericksViewModelFactory
 import im.vector.app.core.extensions.singletonEntryPoint
@@ -68,7 +68,7 @@ class HomeDetailViewModel @AssistedInject constructor(
         private val vectorDataStore: VectorDataStore,
         private val callManager: WebRtcCallManager,
         private val directRoomHelper: DirectRoomHelper,
-        private val appStateHandler: AppStateHandler,
+        private val spaceStateHandler: SpaceStateHandler,
         private val autoAcceptInvites: AutoAcceptInvites,
         private val vectorOverrides: VectorOverrides
 ) : VectorViewModel<HomeDetailViewState, HomeDetailAction, HomeDetailViewEvents>(initialState),
@@ -206,7 +206,7 @@ class HomeDetailViewModel @AssistedInject constructor(
     }
 
     private fun observeRoomGroupingMethod() {
-        appStateHandler.selectedSpaceFlow
+        spaceStateHandler.getSelectedSpaceFlow()
                 .setOnEach {
                     copy(
                             selectedSpace = it.orNull()
@@ -215,7 +215,7 @@ class HomeDetailViewModel @AssistedInject constructor(
     }
 
     private fun observeRoomSummaries() {
-        appStateHandler.selectedSpaceFlow.distinctUntilChanged().flatMapLatest {
+        spaceStateHandler.getSelectedSpaceFlow().distinctUntilChanged().flatMapLatest {
             // we use it as a trigger to all changes in room, but do not really load
             // the actual models
             session.roomService().getPagedRoomSummariesLive(
@@ -227,7 +227,7 @@ class HomeDetailViewModel @AssistedInject constructor(
         }
                 .throttleFirst(300)
                 .onEach {
-                    val activeSpaceRoomId = appStateHandler.getCurrentSpace()?.roomId
+                    val activeSpaceRoomId = spaceStateHandler.getCurrentSpace()?.roomId
                     var dmInvites = 0
                     var roomsInvite = 0
                     if (autoAcceptInvites.showInvites()) {

vector/src/main/java/im/vector/app/features/home/UnreadMessagesSharedViewModel.kt

@@ -22,7 +22,7 @@ import com.airbnb.mvrx.MavericksViewModelFactory
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
-import im.vector.app.AppStateHandler
+import im.vector.app.SpaceStateHandler
 import im.vector.app.core.di.MavericksAssistedViewModelFactory
 import im.vector.app.core.di.hiltMavericksViewModelFactory
 import im.vector.app.core.platform.EmptyAction
@@ -58,7 +58,7 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor(
         @Assisted initialState: UnreadMessagesState,
         session: Session,
         private val vectorPreferences: VectorPreferences,
-        appStateHandler: AppStateHandler,
+        spaceStateHandler: SpaceStateHandler,
         private val autoAcceptInvites: AutoAcceptInvites
 ) :
         VectorViewModel<UnreadMessagesState, EmptyAction, EmptyViewEvents>(initialState) {
@@ -109,8 +109,8 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor(
                 }
 
         combine(
-                appStateHandler.selectedSpaceFlow.distinctUntilChanged(),
-                appStateHandler.selectedSpaceFlow.flatMapLatest {
+                spaceStateHandler.getSelectedSpaceFlow().distinctUntilChanged(),
+                spaceStateHandler.getSelectedSpaceFlow().flatMapLatest {
                     roomService.getPagedRoomSummariesLive(
                             roomSummaryQueryParams {
                                 this.memberships = Membership.activeMemberships()
@@ -161,10 +161,10 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor(
             CountInfo(
                     homeCount = counts,
                     otherCount = RoomAggregateNotificationCount(
-                            notificationCount = rootCounts.fold(0, { acc, rs -> acc + rs.notificationCount }) +
+                            notificationCount = rootCounts.fold(0) { acc, rs -> acc + rs.notificationCount } +
                                     (counts.notificationCount.takeIf { selectedSpace != null } ?: 0) +
                                     spaceInviteCount,
-                            highlightCount = rootCounts.fold(0, { acc, rs -> acc + rs.highlightCount }) +
+                            highlightCount = rootCounts.fold(0) { acc, rs -> acc + rs.highlightCount } +
                                     (counts.highlightCount.takeIf { selectedSpace != null } ?: 0) +
                                     spaceInviteCount
                     )