-
Notifications
You must be signed in to change notification settings - Fork 175
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
Create SyncOrchestrator
#4176
Open
jmartinesp
wants to merge
9
commits into
develop
Choose a base branch
from
refactor/sync-observer
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+936
−324
Open
Create SyncOrchestrator
#4176
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
35aeec4
Create `SyncObserver` to centralise the sync start/stop flow through …
jmartinesp 19ab314
Use the right reason to set the app as enabled when receiving a push …
jmartinesp 705521f
Fix related tests, add docs and improve the code a bit
jmartinesp 2f26b78
Fix tons of lint issues
jmartinesp fc868a8
Add tests for `SyncObserver`, make sure we can stop it when we want to
jmartinesp 801dd62
Minor improvements to logs and docs
jmartinesp b61677a
Rename `SyncObserver` to `SyncOrchestrator`
jmartinesp a86b267
Introduce `SyncOrchestratorProvider` to be able to get a `SyncOrchest…
jmartinesp 3033b67
Make `MatrixClientsHolder` implement `SyncOrchestratorProvider` too a…
jmartinesp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
135 changes: 135 additions & 0 deletions
135
appnav/src/main/kotlin/io/element/android/appnav/SyncOrchestrator.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| /* | ||
| * 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 | ||
|
|
||
| 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.di.SessionScope | ||
| import io.element.android.libraries.di.SingleIn | ||
| import io.element.android.libraries.di.annotations.SessionCoroutineScope | ||
| 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 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 javax.inject.Inject | ||
| import kotlin.time.Duration.Companion.milliseconds | ||
| import kotlin.time.Duration.Companion.seconds | ||
|
|
||
| /** | ||
| * Observes the app state and network state to start/stop the sync service. | ||
| */ | ||
| @SingleIn(SessionScope::class) | ||
| class SyncOrchestrator @Inject constructor( | ||
| matrixClient: MatrixClient, | ||
| private val appForegroundStateService: AppForegroundStateService, | ||
| private val networkMonitor: NetworkMonitor, | ||
| @SessionCoroutineScope private val sessionCoroutineScope: CoroutineScope, | ||
| private val dispatchers: CoroutineDispatchers, | ||
| ) { | ||
| private val syncService = matrixClient.syncService() | ||
|
|
||
| private val initialSyncMutex = Mutex() | ||
|
|
||
| private var coroutineScope: CoroutineScope? = null | ||
|
|
||
| private val tag = "SyncOrchestrator" | ||
|
|
||
| /** | ||
| * 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) | ||
| fun start() { | ||
| Timber.tag(tag).d("start observing the app and network state") | ||
|
|
||
| if (syncService.syncState.value != SyncState.Running) { | ||
| Timber.tag(tag).d("initial startSync") | ||
| sessionCoroutineScope.launch(dispatchers.io) { | ||
| try { | ||
| initialSyncMutex.lock() | ||
| syncService.startSync() | ||
|
|
||
| // Wait until it's running | ||
| syncService.syncState.first { it == SyncState.Running } | ||
| } finally { | ||
| initialSyncMutex.unlock() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| coroutineScope = CoroutineScope(sessionCoroutineScope.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. | ||
| 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.Online | ||
|
|
||
| Timber.tag(tag).d("isAppActive=$isAppActive, isNetworkAvailable=$isNetworkAvailable") | ||
| if (syncState == SyncState.Running && (!isAppActive || !isNetworkAvailable)) { | ||
| // Don't stop the sync immediately, wait a bit to avoid starting/stopping the sync too often | ||
| delay(3.seconds) | ||
| SyncObserverAction.StopSync | ||
| } else if (syncState != SyncState.Running && isAppActive && isNetworkAvailable) { | ||
| SyncObserverAction.StartSync | ||
| } else { | ||
| SyncObserverAction.NoOp | ||
| } | ||
| } | ||
| .distinctUntilChanged() | ||
| .collect { action -> | ||
| when (action) { | ||
| SyncObserverAction.StartSync -> { | ||
| syncService.startSync() | ||
| } | ||
| SyncObserverAction.StopSync -> { | ||
| syncService.stopSync() | ||
| } | ||
| SyncObserverAction.NoOp -> Unit | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Stop observing the app state and network state. | ||
| */ | ||
| fun stop() { | ||
| Timber.tag(tag).d("stop observing the app and network state") | ||
| coroutineScope?.cancel() | ||
| coroutineScope = null | ||
| } | ||
| } | ||
|
|
||
| private enum class SyncObserverAction { | ||
| StartSync, | ||
| StopSync, | ||
| NoOp, | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this code will not be run if the app is killed and a Push is received. In this case the sync will not start?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right, and I completely overlooked this... This complicates things, as we would have to use
SyncOrchestratorin theAppScopeand that means holding as many asMatrixClients there are inMatrixClientsHolder, maybe following a similar approach 🫤 .