Skip to content

Simplify syncing the room list when receiving a push #4915

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
+7 −62
Open

Simplify syncing the room list when receiving a push #4915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 6 additions & 37 deletions ...impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,22 @@
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(
private val matrixClientProvider: MatrixClientProvider,
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)) {
Expand All @@ -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
Expand All @@ -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
}
}
}
}
}
}
26 changes: 1 addition & 25 deletions .../src/test/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEventTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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))
Expand All @@ -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,
)
Expand All @@ -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()
Expand All @@ -183,7 +161,6 @@ class SyncOnNotifiableEventTest {
appForegroundStateService: FakeAppForegroundStateService = FakeAppForegroundStateService(
initialForegroundValue = true,
),
activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(),
): SyncOnNotifiableEvent {
val featureFlagService = FakeFeatureFlagService(
initialState = mapOf(
Expand All @@ -196,7 +173,6 @@ class SyncOnNotifiableEventTest {
featureFlagService = featureFlagService,
appForegroundStateService = appForegroundStateService,
dispatchers = testCoroutineDispatchers(),
activeRoomsHolder = activeRoomsHolder,
)
}
}
Loading