Simplify syncing the room list when receiving a push (#4915)

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<NotifiableEvent>) = 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) {
+                    // Give the sync some time to complete in background
-                        val activeRoom = activeRoomsHolder.getActiveRoomMatching(sessionId, roomId)
+                    delay(10.seconds)
-                        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()
-                        }
-                    }
                 }
             }
         } 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
-                    }
-                }
-            }
-        }
-    }
 }

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<List<MatrixTimelineItem>>(emptyList())
     private val startSyncLambda = lambdaRecorder<Result<Unit>> { Result.success(Unit) }
     private val stopSyncLambda = lambdaRecorder<Result<Unit>> { Result.success(Unit) }
     private val subscribeToSyncLambda = lambdaRecorder<Unit> { }
 
-    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,
         )
     }
 }