Skip to content

Create SyncOrchestrator #4176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.ui.Modifier
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import com.bumble.appyx.core.composable.PermanentChild
import com.bumble.appyx.core.lifecycle.subscribe
import com.bumble.appyx.core.modality.BuildContext
Expand Down Expand Up @@ -52,8 +50,6 @@ import io.element.android.features.ftue.api.FtueEntryPoint
import io.element.android.features.ftue.api.state.FtueService
import io.element.android.features.ftue.api.state.FtueState
import io.element.android.features.logout.api.LogoutEntryPoint
import io.element.android.features.networkmonitor.api.NetworkMonitor
import io.element.android.features.networkmonitor.api.NetworkStatus
import io.element.android.features.preferences.api.PreferencesEntryPoint
import io.element.android.features.roomdirectory.api.RoomDescription
import io.element.android.features.roomdirectory.api.RoomDirectoryEntryPoint
Expand All @@ -77,18 +73,14 @@ import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.core.toRoomIdOrAlias
import io.element.android.libraries.matrix.api.permalink.PermalinkData
import io.element.android.libraries.matrix.api.sync.SyncState
import io.element.android.libraries.matrix.api.verification.SessionVerificationRequestDetails
import io.element.android.libraries.matrix.api.verification.SessionVerificationServiceListener
import io.element.android.libraries.preferences.api.store.EnableNativeSlidingSyncUseCase
import io.element.android.services.appnavstate.api.AppNavigationStateService
import io.element.android.services.appnavstate.api.SyncOrchestratorProvider
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.debounce
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.launch
import kotlinx.parcelize.Parcelize
import timber.log.Timber
Expand All @@ -107,7 +99,6 @@ class LoggedInFlowNode @AssistedInject constructor(
private val userProfileEntryPoint: UserProfileEntryPoint,
private val ftueEntryPoint: FtueEntryPoint,
private val coroutineScope: CoroutineScope,
private val networkMonitor: NetworkMonitor,
private val ftueService: FtueService,
private val roomDirectoryEntryPoint: RoomDirectoryEntryPoint,
private val shareEntryPoint: ShareEntryPoint,
Expand All @@ -116,6 +107,7 @@ class LoggedInFlowNode @AssistedInject constructor(
private val logoutEntryPoint: LogoutEntryPoint,
private val incomingVerificationEntryPoint: IncomingVerificationEntryPoint,
private val enableNativeSlidingSyncUseCase: EnableNativeSlidingSyncUseCase,
private val syncOrchestratorProvider: SyncOrchestratorProvider,
snackbarDispatcher: SnackbarDispatcher,
) : BaseFlowNode<LoggedInFlowNode.NavTarget>(
backstack = BackStack(
Expand All @@ -133,7 +125,6 @@ class LoggedInFlowNode @AssistedInject constructor(
fun onOpenBugReport()
}

private val syncService = matrixClient.syncService()
private val loggedInFlowProcessor = LoggedInEventProcessor(
snackbarDispatcher,
matrixClient.roomMembershipObserver(),
Expand All @@ -147,6 +138,9 @@ class LoggedInFlowNode @AssistedInject constructor(

override fun onBuilt() {
super.onBuilt()

syncOrchestratorProvider.getSyncOrchestrator(sessionId = matrixClient.sessionId)?.start()

lifecycle.subscribe(
onCreate = {
appNavigationStateService.onNavigateToSession(id, matrixClient.sessionId)
Expand All @@ -165,52 +159,20 @@ class LoggedInFlowNode @AssistedInject constructor(
}
.launchIn(lifecycleScope)
},
onStop = {
coroutineScope.launch {
// Counterpart startSync is done in observeSyncStateAndNetworkStatus method.
syncService.stopSync()
}
},
onDestroy = {
appNavigationStateService.onLeavingSpace(id)
appNavigationStateService.onLeavingSession(id)
loggedInFlowProcessor.stopObserving()
matrixClient.sessionVerificationService().setListener(null)
}
)
observeSyncStateAndNetworkStatus()
setupSendingQueue()
}

private fun setupSendingQueue() {
sendingQueue.launchIn(lifecycleScope)
}

@OptIn(FlowPreview::class)
private fun observeSyncStateAndNetworkStatus() {
lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.STARTED) {
combine(
// small debounce to avoid spamming startSync when the state is changing quickly in case of error.
syncService.syncState.debounce(100),
networkMonitor.connectivity
) { syncState, networkStatus ->
Pair(syncState, networkStatus)
}
.onStart {
// Temporary fix to ensure that the sync is started even if the networkStatus is offline.
syncService.startSync()
}
.collect { (syncState, networkStatus) ->
Timber.d("Sync state: $syncState, network status: $networkStatus")
if (syncState != SyncState.Running && networkStatus == NetworkStatus.Online) {
syncService.startSync()
}
}
}
}
}

sealed interface NavTarget : Parcelable {
@Parcelize
data object Placeholder : NavTarget
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import im.vector.app.features.analytics.plan.JoinedRoom
import io.element.android.anvilannotations.ContributesNode
import io.element.android.appnav.di.MatrixClientsHolder
import io.element.android.appnav.di.MatrixSessionCache
import io.element.android.appnav.intent.IntentResolver
import io.element.android.appnav.intent.ResolvedIntent
import io.element.android.appnav.root.RootNavStateFlowFactory
Expand Down Expand Up @@ -62,7 +62,7 @@ class RootFlowNode @AssistedInject constructor(
@Assisted plugins: List<Plugin>,
private val authenticationService: MatrixAuthenticationService,
private val navStateFlowFactory: RootNavStateFlowFactory,
private val matrixClientsHolder: MatrixClientsHolder,
private val matrixSessionCache: MatrixSessionCache,
private val presenter: RootPresenter,
private val bugReportEntryPoint: BugReportEntryPoint,
private val viewFolderEntryPoint: ViewFolderEntryPoint,
Expand All @@ -78,14 +78,14 @@ class RootFlowNode @AssistedInject constructor(
plugins = plugins
) {
override fun onBuilt() {
matrixClientsHolder.restoreWithSavedState(buildContext.savedStateMap)
matrixSessionCache.restoreWithSavedState(buildContext.savedStateMap)
super.onBuilt()
observeNavState()
}

override fun onSaveInstanceState(state: MutableSavedStateMap) {
super.onSaveInstanceState(state)
matrixClientsHolder.saveIntoSavedState(state)
matrixSessionCache.saveIntoSavedState(state)
navStateFlowFactory.saveIntoSavedState(state)
}

Expand Down Expand Up @@ -118,7 +118,7 @@ class RootFlowNode @AssistedInject constructor(
}

private fun switchToNotLoggedInFlow() {
matrixClientsHolder.removeAll()
matrixSessionCache.removeAll()
backstack.safeRoot(NavTarget.NotLoggedInFlow)
}

Expand All @@ -131,7 +131,7 @@ class RootFlowNode @AssistedInject constructor(
onFailure: () -> Unit,
onSuccess: (SessionId) -> Unit,
) {
matrixClientsHolder.getOrRestore(sessionId)
matrixSessionCache.getOrRestore(sessionId)
.onSuccess {
Timber.v("Succeed to restore session $sessionId")
onSuccess(sessionId)
Expand Down Expand Up @@ -200,7 +200,7 @@ class RootFlowNode @AssistedInject constructor(
override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
return when (navTarget) {
is NavTarget.LoggedInFlow -> {
val matrixClient = matrixClientsHolder.getOrNull(navTarget.sessionId) ?: return splashNode(buildContext).also {
val matrixClient = matrixSessionCache.getOrNull(navTarget.sessionId) ?: return splashNode(buildContext).also {
Timber.w("Couldn't find any session, go through SplashScreen")
}
val inputs = LoggedInAppScopeFlowNode.Inputs(matrixClient)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/

package io.element.android.appnav.di

import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import io.element.android.features.networkmonitor.api.NetworkMonitor
import io.element.android.features.networkmonitor.api.NetworkStatus
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.sync.SyncState
import io.element.android.services.appnavstate.api.AppForegroundStateService
import io.element.android.services.appnavstate.api.SyncOrchestrator
import kotlinx.coroutines.CoroutineName
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.cancel
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.debounce
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import kotlinx.coroutines.sync.Mutex
import timber.log.Timber
import java.util.concurrent.atomic.AtomicBoolean
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds

class DefaultSyncOrchestrator @AssistedInject constructor(
@Assisted matrixClient: MatrixClient,
private val baseCoroutineScope: CoroutineScope = matrixClient.sessionCoroutineScope,
private val appForegroundStateService: AppForegroundStateService,
private val networkMonitor: NetworkMonitor,
private val dispatchers: CoroutineDispatchers,
) : SyncOrchestrator {
@AssistedFactory
interface Factory {
fun create(matrixClient: MatrixClient): DefaultSyncOrchestrator
}

private val syncService = matrixClient.syncService()

private val initialSyncMutex = Mutex()

private var coroutineScope: CoroutineScope? = null

private val tag = "SyncOrchestrator"

private val started = AtomicBoolean(false)

/**
* Starting observing the app state and network state to start/stop the sync service.
*
* Before observing the state, a first attempt at starting the sync service will happen if it's not already running.
*/
@OptIn(FlowPreview::class)
override fun start() {
if (!started.compareAndSet(false, true)) {
Timber.tag(tag).d("already started, exiting early")

Check warning on line 66 in appnav/src/main/kotlin/io/element/android/appnav/di/DefaultSyncOrchestrator.kt

View check run for this annotation

Codecov / codecov/patch

appnav/src/main/kotlin/io/element/android/appnav/di/DefaultSyncOrchestrator.kt#L66

Added line #L66 was not covered by tests
return
}

Timber.tag(tag).d("start observing the app and network state")

if (syncService.syncState.value != SyncState.Running) {
Timber.tag(tag).d("initial startSync")
baseCoroutineScope.launch(dispatchers.io) {
try {
initialSyncMutex.lock()
syncService.startSync()

// Wait until it's running
syncService.syncState.first { it == SyncState.Running }
} finally {
initialSyncMutex.unlock()
}
}
}

coroutineScope = CoroutineScope(baseCoroutineScope.coroutineContext + CoroutineName(tag) + dispatchers.io)

coroutineScope?.launch {
// Wait until the initial sync is done, either successfully or failing
initialSyncMutex.lock()

combine(
// small debounce to avoid spamming startSync when the state is changing quickly in case of error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the debounced can be moved after the combine block (before .distinctUntilChanged())?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes issues in the tests, since it's preventing some intermediate states from being generated. I'd rather keep it were it is, if possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try with this patch?

moveDebounce.patch

syncService.syncState.debounce(100.milliseconds),
networkMonitor.connectivity,
appForegroundStateService.isInForeground,
appForegroundStateService.isInCall,
appForegroundStateService.isSyncingNotificationEvent,
) { syncState, networkState, isInForeground, isInCall, isSyncingNotificationEvent ->
val isAppActive = isInForeground || isInCall || isSyncingNotificationEvent
val isNetworkAvailable = networkState == NetworkStatus.Connected

Timber.tag(tag).d("isAppActive=$isAppActive, isNetworkAvailable=$isNetworkAvailable")
if (syncState == SyncState.Running && !isAppActive) {
// Don't stop the sync immediately, wait a bit to avoid starting/stopping the sync too often
delay(3.seconds)
SyncStateAction.StopSync
} else if (syncState != SyncState.Running && isAppActive && isNetworkAvailable) {
SyncStateAction.StartSync
} else {
SyncStateAction.NoOp
}
}
.distinctUntilChanged()
.collect { action ->
when (action) {
SyncStateAction.StartSync -> {
syncService.startSync()
}
SyncStateAction.StopSync -> {
syncService.stopSync()
}
SyncStateAction.NoOp -> Unit
}
}
}
}

/**
* Stop observing the app state and network state.
*/
override fun stop() {
if (!started.compareAndSet(true, false)) {
Timber.tag(tag).d("already stopped, exiting early")

Check warning on line 135 in appnav/src/main/kotlin/io/element/android/appnav/di/DefaultSyncOrchestrator.kt

View check run for this annotation

Codecov / codecov/patch

appnav/src/main/kotlin/io/element/android/appnav/di/DefaultSyncOrchestrator.kt#L135

Added line #L135 was not covered by tests
return
}
Timber.tag(tag).d("stop observing the app and network state")
coroutineScope?.cancel()
coroutineScope = null
}
}

private enum class SyncStateAction {
StartSync,
StopSync,
NoOp,
}
Loading
Loading