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,41 @@
+/*
+ * 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
+
+interface SpaceStateHandler : DefaultLifecycleObserver {
+
+    fun getCurrentSpace(): RoomSummary?
+
+    fun setCurrentSpace(
+            spaceId: String?,
+            session: Session? = null,
+            persistNow: Boolean = false,
+            isForwardNavigation: Boolean = true,
+    )
+
+    fun getSpaceBackstack(): ArrayDeque<String?>
+
+    fun getSelectedSpaceFlow(): Flow<Option<RoomSummary>>
+
+    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 androidx.lifecycle.asFlow
 import arrow.core.Option
@@ -47,54 +46,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()
                 }
             }
         }
@@ -124,9 +129,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

@@ -90,7 +90,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
@@ -187,7 +187,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
@@ -129,7 +129,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
 

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
@@ -66,7 +66,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
 ) : VectorBaseFragment<FragmentHomeDetailBinding>(),
         KeysBackupBanner.Delegate,
         CurrentCallsView.Callback,
@@ -183,13 +183,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)
     }
 
@@ -212,7 +212,7 @@ class HomeDetailFragment @Inject constructor(
     }
 
     private fun refreshSpaceState() {
-        appStateHandler.getCurrentSpace()?.let {
+        spaceStateHandler.getCurrentSpace()?.let {
             onSpaceChange(it)
         }
     }
@@ -466,7 +466,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),
@@ -207,7 +207,7 @@ class HomeDetailViewModel @AssistedInject constructor(
     }
 
     private fun observeRoomGroupingMethod() {
-        appStateHandler.selectedSpaceFlow
+        spaceStateHandler.getSelectedSpaceFlow()
                 .setOnEach {
                     copy(
                             selectedSpace = it.orNull()
@@ -216,7 +216,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(
@@ -228,7 +228,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
                     )

vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt

@@ -28,8 +28,8 @@ import com.airbnb.mvrx.Uninitialized
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
-import im.vector.app.AppStateHandler
 import im.vector.app.R
+import im.vector.app.SpaceStateHandler
 import im.vector.app.core.di.MavericksAssistedViewModelFactory
 import im.vector.app.core.di.hiltMavericksViewModelFactory
 import im.vector.app.core.mvrx.runCatchingToAsync
@@ -139,7 +139,7 @@ class TimelineViewModel @AssistedInject constructor(
         private val stopLiveLocationShareUseCase: StopLiveLocationShareUseCase,
         private val redactLiveLocationShareEventUseCase: RedactLiveLocationShareEventUseCase,
         timelineFactory: TimelineFactory,
-        appStateHandler: AppStateHandler,
+        spaceStateHandler: SpaceStateHandler,
 ) : VectorViewModel<RoomDetailViewState, RoomDetailAction, RoomDetailViewEvents>(initialState),
         Timeline.Listener, ChatEffectManager.Delegate, CallProtocolsChecker.Listener, LocationSharingServiceConnection.Callback {
 
@@ -221,16 +221,16 @@ class TimelineViewModel @AssistedInject constructor(
         if (initialState.switchToParentSpace) {
             // We are coming from a notification, try to switch to the most relevant space
             // so that when hitting back the room will appear in the list
-            appStateHandler.getCurrentSpace().let { currentSpace ->
+            spaceStateHandler.getCurrentSpace().let { currentSpace ->
                 val currentRoomSummary = room.roomSummary() ?: return@let
                 // nothing we are good
                 if ((currentSpace == null && !vectorPreferences.prefSpacesShowAllRoomInHome()) ||
                         (currentSpace != null && !currentRoomSummary.flattenParentIds.contains(currentSpace.roomId))) {
                     // take first one or switch to home
-                    appStateHandler.setCurrentSpace(
+                    spaceStateHandler.setCurrentSpace(
                             currentRoomSummary
                                     .flattenParentIds.firstOrNull { it.isNotBlank() },
-                            // force persist, because if not on resume the AppStateHandler will resume
+                            // force persist, because if not on resume the SpaceStateHandler will resume
                             // the current space from what was persisted on enter background
                             persistNow = true
                     )