From 6c0287a2fb543ecfefd006ec9f7e8864ee552ae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Mon, 23 Jun 2025 11:29:18 +0200 Subject: [PATCH] Simplify syncing the room list when receiving a push We were previously creating a timeline for every room that received a push notification and waiting until the event was resolved for it before stopping the sync. Creating this timeline could be causing unexpected UTDs, so we'll just give the sync a few seconds to run in background instead. --- .../push/impl/push/SyncOnNotifiableEvent.kt | 43 +++---------------- .../impl/push/SyncOnNotifiableEventTest.kt | 26 +---------- 2 files changed, 7 insertions(+), 62 deletions(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt index 961b5ebf9d1..57c004ce234 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt @@ -8,21 +8,15 @@ package io.element.android.libraries.push.impl.push import io.element.android.libraries.core.coroutine.CoroutineDispatchers -import io.element.android.libraries.core.coroutine.parallelMap import io.element.android.libraries.featureflag.api.FeatureFlagService import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.matrix.api.MatrixClientProvider -import io.element.android.libraries.matrix.api.core.EventId -import io.element.android.libraries.matrix.api.room.JoinedRoom -import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent -import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.services.appnavstate.api.AppForegroundStateService -import kotlinx.coroutines.flow.first +import kotlinx.coroutines.delay import kotlinx.coroutines.withContext -import kotlinx.coroutines.withTimeoutOrNull +import timber.log.Timber import javax.inject.Inject -import kotlin.time.Duration import kotlin.time.Duration.Companion.seconds class SyncOnNotifiableEvent @Inject constructor( @@ -30,7 +24,6 @@ class SyncOnNotifiableEvent @Inject constructor( private val featureFlagService: FeatureFlagService, private val appForegroundStateService: AppForegroundStateService, private val dispatchers: CoroutineDispatchers, - private val activeRoomsHolder: ActiveRoomsHolder, ) { suspend operator fun invoke(notifiableEvents: List) = withContext(dispatchers.io) { if (!featureFlagService.isFeatureEnabled(FeatureFlags.SyncOnPush)) { @@ -41,6 +34,7 @@ class SyncOnNotifiableEvent @Inject constructor( val eventsBySession = notifiableEvents.groupBy { it.sessionId } appForegroundStateService.updateIsSyncingNotificationEvent(true) + Timber.d("Starting opportunistic room list sync | In foreground: ${appForegroundStateService.isInForeground.value}") for ((sessionId, events) in eventsBySession) { val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: continue @@ -49,38 +43,13 @@ class SyncOnNotifiableEvent @Inject constructor( client.roomListService.subscribeToVisibleRooms(eventsByRoomId.keys.toList()) if (!appForegroundStateService.isInForeground.value) { - for ((roomId, eventsInRoom) in eventsByRoomId) { - val activeRoom = activeRoomsHolder.getActiveRoomMatching(sessionId, roomId) - val room = activeRoom ?: client.getJoinedRoom(roomId) - - if (room != null) { - eventsInRoom.parallelMap { event -> - room.waitsUntilEventIsKnown(event.eventId, timeout = 10.seconds) - } - } - - if (room != null && activeRoom == null) { - // Destroy the room we just instantiated to reset its live timeline - room.destroy() - } - } + // Give the sync some time to complete in background + delay(10.seconds) } } } finally { + Timber.d("Finished opportunistic room list sync") appForegroundStateService.updateIsSyncingNotificationEvent(false) } } - - private suspend fun JoinedRoom.waitsUntilEventIsKnown(eventId: EventId, timeout: Duration) { - withTimeoutOrNull(timeout) { - liveTimeline.timelineItems.first { timelineItems -> - timelineItems.any { timelineItem -> - when (timelineItem) { - is MatrixTimelineItem.Event -> timelineItem.eventId == eventId - else -> false - } - } - } - } - } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEventTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEventTest.kt index 8ad5e28e6ed..e1c4b33acf5 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEventTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEventTest.kt @@ -13,27 +13,20 @@ import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.test.FakeFeatureFlagService import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.sync.SyncState -import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.test.A_ROOM_ID -import io.element.android.libraries.matrix.test.A_UNIQUE_ID import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.FakeMatrixClientProvider import io.element.android.libraries.matrix.test.room.FakeBaseRoom import io.element.android.libraries.matrix.test.room.FakeJoinedRoom import io.element.android.libraries.matrix.test.room.aRoomInfo import io.element.android.libraries.matrix.test.sync.FakeSyncService -import io.element.android.libraries.matrix.test.timeline.FakeTimeline -import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem import io.element.android.libraries.push.impl.notifications.fixtures.aNotifiableCallEvent import io.element.android.libraries.push.impl.notifications.fixtures.aNotifiableMessageEvent -import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.services.appnavstate.test.FakeAppForegroundStateService import io.element.android.tests.testutils.lambda.assert import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceTimeBy @@ -43,16 +36,11 @@ import java.util.concurrent.atomic.AtomicBoolean import kotlin.time.Duration.Companion.seconds class SyncOnNotifiableEventTest { - private val timelineItems = MutableStateFlow>(emptyList()) private val startSyncLambda = lambdaRecorder> { Result.success(Unit) } private val stopSyncLambda = lambdaRecorder> { Result.success(Unit) } private val subscribeToSyncLambda = lambdaRecorder { } - private val liveTimeline = FakeTimeline( - timelineItems = timelineItems, - ) private val room = FakeJoinedRoom( - liveTimeline = liveTimeline, baseRoom = FakeBaseRoom( roomId = A_ROOM_ID, subscribeToSyncLambda = subscribeToSyncLambda, @@ -130,10 +118,6 @@ class SyncOnNotifiableEventTest { ) val sut = createSyncOnNotifiableEvent(client = client, appForegroundStateService = appForegroundStateService, isSyncOnPushEnabled = true) - timelineItems.emit( - listOf(MatrixTimelineItem.Event(A_UNIQUE_ID, anEventTimelineItem())) - ) - appForegroundStateService.isSyncingNotificationEvent.test { syncService.emitSyncState(SyncState.Running) sut(listOf(notifiableEvent)) @@ -150,7 +134,7 @@ class SyncOnNotifiableEventTest { } @Test - fun `when feature flag is enabled and app is in background, running multiple time only call once`() = runTest { + fun `when feature flag is enabled and app is in background, running multiple times only call once`() = runTest { val appForegroundStateService = FakeAppForegroundStateService( initialForegroundValue = false, ) @@ -159,12 +143,6 @@ class SyncOnNotifiableEventTest { appForegroundStateService.isSyncingNotificationEvent.test { launch { sut(listOf(notifiableEvent)) } launch { sut(listOf(notifiableEvent)) } - launch { - delay(1) - timelineItems.emit( - listOf(MatrixTimelineItem.Event(A_UNIQUE_ID, anEventTimelineItem())) - ) - } // It's initially false assertThat(awaitItem()).isFalse() @@ -183,7 +161,6 @@ class SyncOnNotifiableEventTest { appForegroundStateService: FakeAppForegroundStateService = FakeAppForegroundStateService( initialForegroundValue = true, ), - activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(), ): SyncOnNotifiableEvent { val featureFlagService = FakeFeatureFlagService( initialState = mapOf( @@ -196,7 +173,6 @@ class SyncOnNotifiableEventTest { featureFlagService = featureFlagService, appForegroundStateService = appForegroundStateService, dispatchers = testCoroutineDispatchers(), - activeRoomsHolder = activeRoomsHolder, ) } }