Make 'room list catch-up' analytics transaction network aware (#6233)
* Make 'room list catch-up' analytics transaction network aware. * Add `RoomListService.isInitialSyncDone`. Use this to simplify `DefaultAnalyticsRoomListStateWatcher`'s logic.
This commit is contained in:
Jorge Martin Espinosa
GitHub
parent
b65b9eeab9
commit
8fab22ec7d
9 changed files with 91 additions and 38 deletions
|
|
@ -8,29 +8,32 @@
|
|||
package io.element.android.services.analytics.impl.watchers
|
||||
|
||||
import dev.zacsweers.metro.ContributesBinding
|
||||
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.core.coroutine.childScope
|
||||
import io.element.android.libraries.core.coroutine.withPreviousValue
|
||||
import io.element.android.libraries.di.SessionScope
|
||||
import io.element.android.libraries.di.annotations.SessionCoroutineScope
|
||||
import io.element.android.libraries.matrix.api.roomlist.RoomListService
|
||||
import io.element.android.services.analytics.api.AnalyticsLongRunningTransaction
|
||||
import io.element.android.services.analytics.api.AnalyticsService
|
||||
import io.element.android.services.analytics.api.finishLongRunningTransaction
|
||||
import io.element.android.services.analytics.api.watchers.AnalyticsRoomListStateWatcher
|
||||
import io.element.android.services.appnavstate.api.AppNavigationStateService
|
||||
import io.element.android.services.appnavstate.api.AppForegroundStateService
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.cancelChildren
|
||||
import kotlinx.coroutines.flow.combine
|
||||
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||
import kotlinx.coroutines.flow.launchIn
|
||||
import kotlinx.coroutines.flow.map
|
||||
import kotlinx.coroutines.flow.onEach
|
||||
import timber.log.Timber
|
||||
import java.util.concurrent.atomic.AtomicBoolean
|
||||
import kotlin.time.Duration.Companion.minutes
|
||||
|
||||
@ContributesBinding(SessionScope::class)
|
||||
class DefaultAnalyticsRoomListStateWatcher(
|
||||
private val appNavigationStateService: AppNavigationStateService,
|
||||
private val appForegroundStateService: AppForegroundStateService,
|
||||
private val networkMonitor: NetworkMonitor,
|
||||
private val roomListService: RoomListService,
|
||||
private val analyticsService: AnalyticsService,
|
||||
@SessionCoroutineScope sessionCoroutineScope: CoroutineScope,
|
||||
|
|
@ -38,7 +41,7 @@ class DefaultAnalyticsRoomListStateWatcher(
|
|||
) : AnalyticsRoomListStateWatcher {
|
||||
private val coroutineScope: CoroutineScope = sessionCoroutineScope.childScope(dispatchers.computation, "AnalyticsRoomListStateWatcher")
|
||||
private val isStarted = AtomicBoolean(false)
|
||||
private val isWarmState = AtomicBoolean(false)
|
||||
private val isNotInitialSync get() = roomListService.isInitialSyncDone
|
||||
|
||||
override fun start() {
|
||||
if (isStarted.getAndSet(true)) {
|
||||
|
|
@ -48,27 +51,40 @@ class DefaultAnalyticsRoomListStateWatcher(
|
|||
|
||||
val longRunningTransaction = AnalyticsLongRunningTransaction.CatchUp
|
||||
|
||||
appNavigationStateService.appNavigationState
|
||||
.map { it.isInForeground }
|
||||
val hasNetworkConnectivityFlow = networkMonitor.connectivity
|
||||
.map { it == NetworkStatus.Connected }
|
||||
.distinctUntilChanged()
|
||||
.withPreviousValue()
|
||||
.onEach { (wasInForeground, isInForeground) ->
|
||||
if (isInForeground && roomListService.state.value != RoomListService.State.Running) {
|
||||
analyticsService.startLongRunningTransaction(longRunningTransaction)
|
||||
} else if (!isInForeground) {
|
||||
analyticsService.removeLongRunningTransaction(longRunningTransaction)
|
||||
}
|
||||
|
||||
if (wasInForeground == false && isInForeground) {
|
||||
isWarmState.set(true)
|
||||
combine(
|
||||
appForegroundStateService.isInForeground,
|
||||
hasNetworkConnectivityFlow,
|
||||
) { isInForeground, hasNetworkConnectivity ->
|
||||
val canSync = isInForeground && hasNetworkConnectivity
|
||||
val isNotSyncing = roomListService.state.value != RoomListService.State.Running
|
||||
if (isNotInitialSync && canSync && isNotSyncing) {
|
||||
Timber.d("Catch-up transaction: starting")
|
||||
analyticsService.startLongRunningTransaction(longRunningTransaction)
|
||||
} else if (!isInForeground || !hasNetworkConnectivity) {
|
||||
analyticsService.removeLongRunningTransaction(longRunningTransaction)?.let {
|
||||
Timber.d("Catch-up transaction: stopping")
|
||||
}
|
||||
}
|
||||
}
|
||||
.launchIn(coroutineScope)
|
||||
|
||||
roomListService.state
|
||||
.onEach { state ->
|
||||
if (state == RoomListService.State.Running && isWarmState.get()) {
|
||||
analyticsService.finishLongRunningTransaction(longRunningTransaction)
|
||||
if (state == RoomListService.State.Running && isNotInitialSync) {
|
||||
val transaction = analyticsService.removeLongRunningTransaction(longRunningTransaction)
|
||||
if (transaction != null && !transaction.isFinished()) {
|
||||
val duration = transaction.duration
|
||||
if (duration > 3.minutes) {
|
||||
Timber.d("Cancelling catch-up transaction, the elapsed time is too long ($duration), something probably went wrong while measuring")
|
||||
} else {
|
||||
Timber.d("Catch-up transaction finished in $duration")
|
||||
transaction.finish()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
.launchIn(coroutineScope)
|
||||
|
|
|
|||
|
|
@ -8,13 +8,12 @@
|
|||
package io.element.android.services.analytics.impl.watchers
|
||||
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.features.networkmonitor.test.FakeNetworkMonitor
|
||||
import io.element.android.libraries.matrix.api.roomlist.RoomListService
|
||||
import io.element.android.libraries.matrix.test.roomlist.FakeRoomListService
|
||||
import io.element.android.services.analytics.api.AnalyticsLongRunningTransaction.CatchUp
|
||||
import io.element.android.services.analytics.test.FakeAnalyticsService
|
||||
import io.element.android.services.appnavstate.api.AppNavigationState
|
||||
import io.element.android.services.appnavstate.api.NavigationState
|
||||
import io.element.android.services.appnavstate.test.FakeAppNavigationStateService
|
||||
import io.element.android.services.appnavstate.test.FakeAppForegroundStateService
|
||||
import io.element.android.tests.testutils.testCoroutineDispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.test.TestScope
|
||||
|
|
@ -26,13 +25,13 @@ import org.junit.Test
|
|||
class DefaultAnalyticsRoomListStateWatcherTest {
|
||||
@Test
|
||||
fun `Opening the app in a warm state tracks the time until the room list is synced`() = runTest {
|
||||
val navigationStateService = FakeAppNavigationStateService()
|
||||
val appForegroundStateService = FakeAppForegroundStateService()
|
||||
val roomListService = FakeRoomListService().apply {
|
||||
postState(RoomListService.State.Idle)
|
||||
}
|
||||
val analyticsService = FakeAnalyticsService()
|
||||
val watcher = createAnalyticsRoomListStateWatcher(
|
||||
appNavigationStateService = navigationStateService,
|
||||
appForegroundStateService = appForegroundStateService,
|
||||
roomListService = roomListService,
|
||||
analyticsService = analyticsService,
|
||||
)
|
||||
|
|
@ -43,9 +42,9 @@ class DefaultAnalyticsRoomListStateWatcherTest {
|
|||
runCurrent()
|
||||
|
||||
// Make sure it's warm by changing its internal state
|
||||
navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = false))
|
||||
appForegroundStateService.givenIsInForeground(false)
|
||||
runCurrent()
|
||||
navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = true))
|
||||
appForegroundStateService.givenIsInForeground(true)
|
||||
runCurrent()
|
||||
|
||||
// The transaction should be present now
|
||||
|
|
@ -63,15 +62,15 @@ class DefaultAnalyticsRoomListStateWatcherTest {
|
|||
|
||||
@Test
|
||||
fun `Opening the app in a cold state does nothing`() = runTest {
|
||||
val navigationStateService = FakeAppNavigationStateService(
|
||||
initialAppNavigationState = AppNavigationState(NavigationState.Root, false)
|
||||
val appForegroundStateService = FakeAppForegroundStateService(
|
||||
initialForegroundValue = false
|
||||
)
|
||||
val roomListService = FakeRoomListService().apply {
|
||||
postState(RoomListService.State.Idle)
|
||||
}
|
||||
val analyticsService = FakeAnalyticsService()
|
||||
val watcher = createAnalyticsRoomListStateWatcher(
|
||||
appNavigationStateService = navigationStateService,
|
||||
appForegroundStateService = appForegroundStateService,
|
||||
roomListService = roomListService,
|
||||
analyticsService = analyticsService,
|
||||
)
|
||||
|
|
@ -93,13 +92,13 @@ class DefaultAnalyticsRoomListStateWatcherTest {
|
|||
|
||||
@Test
|
||||
fun `The transaction won't be finished until the room list is synchronised`() = runTest {
|
||||
val navigationStateService = FakeAppNavigationStateService()
|
||||
val appForegroundStateService = FakeAppForegroundStateService()
|
||||
val roomListService = FakeRoomListService().apply {
|
||||
postState(RoomListService.State.Idle)
|
||||
}
|
||||
val analyticsService = FakeAnalyticsService()
|
||||
val watcher = createAnalyticsRoomListStateWatcher(
|
||||
appNavigationStateService = navigationStateService,
|
||||
appForegroundStateService = appForegroundStateService,
|
||||
roomListService = roomListService,
|
||||
analyticsService = analyticsService,
|
||||
)
|
||||
|
|
@ -110,9 +109,9 @@ class DefaultAnalyticsRoomListStateWatcherTest {
|
|||
runCurrent()
|
||||
|
||||
// Make sure it's warm by changing its internal state
|
||||
navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = false))
|
||||
appForegroundStateService.givenIsInForeground(false)
|
||||
runCurrent()
|
||||
navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = true))
|
||||
appForegroundStateService.givenIsInForeground(true)
|
||||
runCurrent()
|
||||
|
||||
// The transaction should be present now
|
||||
|
|
@ -128,13 +127,13 @@ class DefaultAnalyticsRoomListStateWatcherTest {
|
|||
|
||||
@Test
|
||||
fun `Opening the app when the room list state was already Running does nothing`() = runTest {
|
||||
val navigationStateService = FakeAppNavigationStateService()
|
||||
val appForegroundStateService = FakeAppForegroundStateService()
|
||||
val roomListService = FakeRoomListService().apply {
|
||||
postState(RoomListService.State.Running)
|
||||
}
|
||||
val analyticsService = FakeAnalyticsService()
|
||||
val watcher = createAnalyticsRoomListStateWatcher(
|
||||
appNavigationStateService = navigationStateService,
|
||||
appForegroundStateService = appForegroundStateService,
|
||||
roomListService = roomListService,
|
||||
analyticsService = analyticsService,
|
||||
)
|
||||
|
|
@ -145,9 +144,9 @@ class DefaultAnalyticsRoomListStateWatcherTest {
|
|||
runCurrent()
|
||||
|
||||
// Make sure it's warm by changing its internal state
|
||||
navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = false))
|
||||
appForegroundStateService.givenIsInForeground(false)
|
||||
runCurrent()
|
||||
navigationStateService.emitNavigationState(AppNavigationState(navigationState = NavigationState.Root, isInForeground = true))
|
||||
appForegroundStateService.givenIsInForeground(true)
|
||||
runCurrent()
|
||||
|
||||
// The transaction was never added
|
||||
|
|
@ -157,14 +156,16 @@ class DefaultAnalyticsRoomListStateWatcherTest {
|
|||
}
|
||||
|
||||
private fun TestScope.createAnalyticsRoomListStateWatcher(
|
||||
appNavigationStateService: FakeAppNavigationStateService = FakeAppNavigationStateService(),
|
||||
appForegroundStateService: FakeAppForegroundStateService = FakeAppForegroundStateService(),
|
||||
roomListService: FakeRoomListService = FakeRoomListService(),
|
||||
analyticsService: FakeAnalyticsService = FakeAnalyticsService(),
|
||||
networkMonitor: FakeNetworkMonitor = FakeNetworkMonitor(),
|
||||
) = DefaultAnalyticsRoomListStateWatcher(
|
||||
appNavigationStateService = appNavigationStateService,
|
||||
appForegroundStateService = appForegroundStateService,
|
||||
roomListService = roomListService,
|
||||
analyticsService = analyticsService,
|
||||
sessionCoroutineScope = backgroundScope,
|
||||
dispatchers = testCoroutineDispatchers(),
|
||||
networkMonitor = networkMonitor,
|
||||
)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue