From 1329daf143ad38bc8c3a4dcde69ef19ea09fcf55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 21 May 2025 14:11:51 +0200 Subject: [PATCH 1/9] Add ActiveRoomHolder to manage active rooms for sessions This allows us to reuse these rooms for several actions that would otherwise create a new instance of the room, which, if the room is already available, would cause delays of several seconds since the live timeline can take a while to restore if it contains many events. --- .../room/joined/JoinedRoomLoadedFlowNode.kt | 4 +++ .../call/impl/ui/CallScreenPresenter.kt | 11 ++++-- .../impl/utils/DefaultCallWidgetProvider.kt | 6 +++- features/preferences/impl/build.gradle.kts | 1 + .../impl/tasks/ClearCacheUseCase.kt | 4 +++ features/share/impl/build.gradle.kts | 1 + .../features/share/impl/SharePresenter.kt | 13 +++++-- .../NotificationBroadcastReceiverHandler.kt | 10 ++++-- .../push/impl/push/SyncOnNotifiableEvent.kt | 8 +++-- .../appnavstate/api/ActiveRoomHolder.kt | 35 +++++++++++++++++++ 10 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt index 094009f3d3a..68c289d11b0 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt @@ -36,6 +36,7 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.permalink.PermalinkData import io.element.android.libraries.matrix.api.room.JoinedRoom +import io.element.android.services.appnavstate.api.ActiveRoomHolder import io.element.android.services.appnavstate.api.AppNavigationStateService import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -51,6 +52,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor( private val appNavigationStateService: AppNavigationStateService, private val appCoroutineScope: CoroutineScope, private val matrixClient: MatrixClient, + private val activeRoomHolder: ActiveRoomHolder, roomComponentFactory: RoomComponentFactory, ) : BaseFlowNode( backstack = BackStack( @@ -85,6 +87,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor( onCreate = { Timber.v("OnCreate => ${inputs.room.roomId}") appNavigationStateService.onNavigateToRoom(id, inputs.room.roomId) + activeRoomHolder.addRoom(inputs.room) fetchRoomMembers() trackVisitedRoom() }, @@ -95,6 +98,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor( }, onDestroy = { Timber.v("OnDestroy") + activeRoomHolder.removeRoom(inputs.room.sessionId) inputs.room.destroy() appNavigationStateService.onLeavingRoom(id) } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt index 7156a3e6038..84dfc4767bc 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt @@ -39,6 +39,7 @@ import io.element.android.libraries.matrix.api.sync.SyncState import io.element.android.libraries.matrix.api.widget.MatrixWidgetDriver import io.element.android.libraries.network.useragent.UserAgentProvider import io.element.android.services.analytics.api.ScreenTracker +import io.element.android.services.appnavstate.api.ActiveRoomHolder import io.element.android.services.appnavstate.api.AppForegroundStateService import io.element.android.services.toolbox.api.systemclock.SystemClock import kotlinx.coroutines.CoroutineScope @@ -62,6 +63,7 @@ class CallScreenPresenter @AssistedInject constructor( private val activeCallManager: ActiveCallManager, private val languageTagProvider: LanguageTagProvider, private val appForegroundStateService: AppForegroundStateService, + private val activeRoomHolder: ActiveRoomHolder, private val appCoroutineScope: CoroutineScope, ) : Presenter { @AssistedFactory @@ -241,8 +243,13 @@ class CallScreenPresenter @AssistedInject constructor( private suspend fun MatrixClient.notifyCallStartIfNeeded(roomId: RoomId) { if (!notifiedCallStart) { - getJoinedRoom(roomId)?.use { it.sendCallNotificationIfNeeded() } - ?.onSuccess { notifiedCallStart = true } + val activeRoomForSession = activeRoomHolder.getActiveRoom(sessionId) + val sendCallNotificationResult = if (activeRoomForSession?.roomId == roomId) { + activeRoomForSession.sendCallNotificationIfNeeded() + } else { + getJoinedRoom(roomId)?.use { it.sendCallNotificationIfNeeded() } + } + sendCallNotificationResult?.onSuccess { notifiedCallStart = true } } } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt index 6f6bd9473c1..9e418bfef92 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt @@ -14,6 +14,7 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.widget.CallWidgetSettingsProvider import io.element.android.libraries.preferences.api.store.AppPreferencesStore +import io.element.android.services.appnavstate.api.ActiveRoomHolder import kotlinx.coroutines.flow.firstOrNull import javax.inject.Inject @@ -24,6 +25,7 @@ class DefaultCallWidgetProvider @Inject constructor( private val matrixClientsProvider: MatrixClientProvider, private val appPreferencesStore: AppPreferencesStore, private val callWidgetSettingsProvider: CallWidgetSettingsProvider, + private val activeRoomHolder: ActiveRoomHolder, ) : CallWidgetProvider { override suspend fun getWidget( sessionId: SessionId, @@ -33,7 +35,9 @@ class DefaultCallWidgetProvider @Inject constructor( theme: String?, ): Result = runCatching { val matrixClient = matrixClientsProvider.getOrRestore(sessionId).getOrThrow() - val room = matrixClient.getJoinedRoom(roomId) ?: error("Room not found") + val room = activeRoomHolder.getActiveRoom(sessionId)?.takeIf { it.roomId == roomId } + ?: matrixClient.getJoinedRoom(roomId) + ?: error("Room not found") val customBaseUrl = appPreferencesStore.getCustomElementCallBaseUrlFlow().firstOrNull() val baseUrl = customBaseUrl ?: EMBEDDED_CALL_WIDGET_BASE_URL diff --git a/features/preferences/impl/build.gradle.kts b/features/preferences/impl/build.gradle.kts index 8d34d55559a..397d0c7acb0 100644 --- a/features/preferences/impl/build.gradle.kts +++ b/features/preferences/impl/build.gradle.kts @@ -78,6 +78,7 @@ dependencies { implementation(projects.features.roomlist.api) implementation(projects.services.analytics.api) implementation(projects.services.analytics.compose) + implementation(projects.services.appnavstate.api) implementation(projects.services.toolbox.api) implementation(libs.datetime) implementation(libs.coil.compose) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt index 8a081c0e417..3a252008f1f 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt @@ -18,6 +18,7 @@ import io.element.android.libraries.di.ApplicationContext import io.element.android.libraries.di.SessionScope import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.push.api.PushService +import io.element.android.services.appnavstate.api.ActiveRoomHolder import kotlinx.coroutines.withContext import okhttp3.OkHttpClient import javax.inject.Inject @@ -37,8 +38,11 @@ class DefaultClearCacheUseCase @Inject constructor( private val ftueService: FtueService, private val pushService: PushService, private val seenInvitesStore: SeenInvitesStore, + private val activeRoomHolder: ActiveRoomHolder, ) : ClearCacheUseCase { override suspend fun invoke() = withContext(coroutineDispatchers.io) { + // Active rooms should be disposed of before clearing the cache + activeRoomHolder.removeRoom(matrixClient.sessionId)?.destroy() // Clear Matrix cache matrixClient.clearCache() // Clear Coil cache diff --git a/features/share/impl/build.gradle.kts b/features/share/impl/build.gradle.kts index ae0cad80dff..c059a249820 100644 --- a/features/share/impl/build.gradle.kts +++ b/features/share/impl/build.gradle.kts @@ -37,6 +37,7 @@ dependencies { implementation(projects.libraries.roomselect.api) implementation(projects.libraries.uiStrings) implementation(projects.libraries.testtags) + implementation(projects.services.appnavstate.api) api(libs.statemachine) api(projects.features.share.api) diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt index 08e9bdc1ff4..4b87163255d 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt @@ -20,9 +20,11 @@ import io.element.android.libraries.architecture.runCatchingUpdatingState import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.RoomId +import io.element.android.libraries.matrix.api.room.JoinedRoom import io.element.android.libraries.mediaupload.api.MediaPreProcessor import io.element.android.libraries.mediaupload.api.MediaSender import io.element.android.libraries.preferences.api.store.SessionPreferencesStore +import io.element.android.services.appnavstate.api.ActiveRoomHolder import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -33,6 +35,7 @@ class SharePresenter @AssistedInject constructor( private val matrixClient: MatrixClient, private val mediaPreProcessor: MediaPreProcessor, private val sessionPreferencesStore: SessionPreferencesStore, + private val activeRoomHolder: ActiveRoomHolder, ) : Presenter { @AssistedFactory interface Factory { @@ -59,6 +62,12 @@ class SharePresenter @AssistedInject constructor( ) } + private suspend fun getJoinedRoom(roomId: RoomId): JoinedRoom? { + return activeRoomHolder.getActiveRoom(matrixClient.sessionId) + ?.takeIf { it.roomId == roomId } + ?: matrixClient.getJoinedRoom(roomId) + } + private fun CoroutineScope.share( intent: Intent, roomIds: List, @@ -72,7 +81,7 @@ class SharePresenter @AssistedInject constructor( } else { roomIds .map { roomId -> - val room = matrixClient.getJoinedRoom(roomId) ?: return@map false + val room = getJoinedRoom(roomId) ?: return@map false val mediaSender = MediaSender( preProcessor = mediaPreProcessor, room = room, @@ -94,7 +103,7 @@ class SharePresenter @AssistedInject constructor( onPlainText = { text -> roomIds .map { roomId -> - matrixClient.getJoinedRoom(roomId)?.liveTimeline?.sendMessage( + getJoinedRoom(roomId)?.liveTimeline?.sendMessage( body = text, htmlBody = null, intentionalMentions = emptyList(), diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt index 9c899c9dfd0..aa228d0dec8 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt @@ -23,6 +23,7 @@ import io.element.android.libraries.push.api.notifications.NotificationCleaner import io.element.android.libraries.push.impl.R import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent import io.element.android.libraries.push.impl.push.OnNotifiableEventReceived +import io.element.android.services.appnavstate.api.ActiveRoomHolder import io.element.android.services.toolbox.api.strings.StringProvider import io.element.android.services.toolbox.api.systemclock.SystemClock import kotlinx.coroutines.CoroutineScope @@ -44,6 +45,7 @@ class NotificationBroadcastReceiverHandler @Inject constructor( private val onNotifiableEventReceived: OnNotifiableEventReceived, private val stringProvider: StringProvider, private val replyMessageExtractor: ReplyMessageExtractor, + private val activeRoomHolder: ActiveRoomHolder, ) { fun onReceive(intent: Intent) { val sessionId = intent.getStringExtra(NotificationBroadcastReceiver.KEY_SESSION_ID)?.let(::SessionId) ?: return @@ -117,13 +119,17 @@ class NotificationBroadcastReceiverHandler @Inject constructor( return@launch } val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: return@launch - client.getJoinedRoom(roomId)?.let { room -> + val room = activeRoomHolder.getActiveRoom(sessionId) + ?.takeIf { it.roomId == roomId } + ?: client.getJoinedRoom(roomId) + + room?.let { sendMatrixEvent( sessionId = sessionId, roomId = roomId, replyToEventId = replyToEventId, threadId = threadId, - room = room, + room = it, message = message, ) } 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 4c5eee260b7..a9e8aa2762e 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 @@ -17,6 +17,7 @@ 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.libraries.push.impl.notifications.model.NotifiableRingingCallEvent +import io.element.android.services.appnavstate.api.ActiveRoomHolder import io.element.android.services.appnavstate.api.AppForegroundStateService import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext @@ -30,15 +31,18 @@ class SyncOnNotifiableEvent @Inject constructor( private val featureFlagService: FeatureFlagService, private val appForegroundStateService: AppForegroundStateService, private val dispatchers: CoroutineDispatchers, + private val activeRoomHolder: ActiveRoomHolder, ) { suspend operator fun invoke(notifiableEvent: NotifiableEvent) = withContext(dispatchers.io) { val isRingingCallEvent = notifiableEvent is NotifiableRingingCallEvent if (!featureFlagService.isFeatureEnabled(FeatureFlags.SyncOnPush) && !isRingingCallEvent) { return@withContext } - val client = matrixClientProvider.getOrRestore(notifiableEvent.sessionId).getOrNull() ?: return@withContext - client.getJoinedRoom(notifiableEvent.roomId)?.use { room -> + val notifiedRoom = activeRoomHolder.getActiveRoom(sessionId = notifiableEvent.sessionId) + ?: matrixClientProvider.getOrRestore(notifiableEvent.sessionId).getOrNull()?.getJoinedRoom(notifiableEvent.roomId) + ?: return@withContext + notifiedRoom.use { room -> room.subscribeToSync() // If the app is in foreground, sync is already running, so we just add the subscription above. diff --git a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt new file mode 100644 index 00000000000..837500bd23f --- /dev/null +++ b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt @@ -0,0 +1,35 @@ +/* + * 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.services.appnavstate.api + +import io.element.android.libraries.di.AppScope +import io.element.android.libraries.di.SingleIn +import io.element.android.libraries.matrix.api.core.SessionId +import io.element.android.libraries.matrix.api.room.JoinedRoom +import java.util.concurrent.ConcurrentHashMap +import javax.inject.Inject + +/** + * Holds the active room for a given session so it can be reused instead of instantiating a new one. + */ +@SingleIn(AppScope::class) +class ActiveRoomHolder @Inject constructor() { + private val rooms = ConcurrentHashMap() + + fun addRoom(room: JoinedRoom) { + rooms[room.sessionId] = room + } + + fun getActiveRoom(sessionId: SessionId): JoinedRoom? { + return rooms[sessionId] + } + + fun removeRoom(sessionId: SessionId): JoinedRoom? { + return rooms.remove(sessionId) + } +} From 2e54b2a50fcebfc7f61b882025a6b2e06f2f1a5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 21 May 2025 14:11:56 +0200 Subject: [PATCH 2/9] Add tests --- ...est.kt => JoinedRoomLoadedFlowNodeTest.kt} | 57 ++++++++++++++++++- .../call/ui/CallScreenPresenterTest.kt | 3 + .../utils/DefaultCallWidgetProviderTest.kt | 21 +++++++ .../tasks/DefaultClearCacheUseCaseTest.kt | 7 +++ .../features/share/impl/SharePresenterTest.kt | 7 ++- ...otificationBroadcastReceiverHandlerTest.kt | 3 + .../impl/push/SyncOnNotifiableEventTest.kt | 5 +- 7 files changed, 99 insertions(+), 4 deletions(-) rename appnav/src/test/kotlin/io/element/android/appnav/{JoinBaseRoomLoadedFlowNodeTest.kt => JoinedRoomLoadedFlowNodeTest.kt} (70%) diff --git a/appnav/src/test/kotlin/io/element/android/appnav/JoinBaseRoomLoadedFlowNodeTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/JoinedRoomLoadedFlowNodeTest.kt similarity index 70% rename from appnav/src/test/kotlin/io/element/android/appnav/JoinBaseRoomLoadedFlowNodeTest.kt rename to appnav/src/test/kotlin/io/element/android/appnav/JoinedRoomLoadedFlowNodeTest.kt index bb15ed6bb98..7b61d677fb0 100644 --- a/appnav/src/test/kotlin/io/element/android/appnav/JoinBaseRoomLoadedFlowNodeTest.kt +++ b/appnav/src/test/kotlin/io/element/android/appnav/JoinedRoomLoadedFlowNodeTest.kt @@ -24,16 +24,18 @@ import io.element.android.features.messages.api.MessagesEntryPoint import io.element.android.features.roomdetails.api.RoomDetailsEntryPoint import io.element.android.libraries.architecture.childNode import io.element.android.libraries.matrix.api.room.JoinedRoom +import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.room.FakeBaseRoom import io.element.android.libraries.matrix.test.room.FakeJoinedRoom +import io.element.android.services.appnavstate.api.ActiveRoomHolder import io.element.android.services.appnavstate.test.FakeAppNavigationStateService import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.test.runTest import org.junit.Rule import org.junit.Test -class JoinBaseRoomLoadedFlowNodeTest { +class JoinedRoomLoadedFlowNodeTest { @get:Rule val instantTaskExecutorRule = InstantTaskExecutorRule() @@ -100,6 +102,7 @@ class JoinBaseRoomLoadedFlowNodeTest { plugins: List, messagesEntryPoint: MessagesEntryPoint = FakeMessagesEntryPoint(), roomDetailsEntryPoint: RoomDetailsEntryPoint = FakeRoomDetailsEntryPoint(), + activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), coroutineScope: CoroutineScope, ) = JoinedRoomLoadedFlowNode( buildContext = BuildContext.root(savedStateMap = null), @@ -110,6 +113,7 @@ class JoinBaseRoomLoadedFlowNodeTest { appCoroutineScope = coroutineScope, roomComponentFactory = FakeRoomComponentFactory(), matrixClient = FakeMatrixClient(), + activeRoomHolder = activeRoomHolder, ) @Test @@ -154,4 +158,55 @@ class JoinBaseRoomLoadedFlowNodeTest { val roomDetailsNode = roomFlowNode.childNode(JoinedRoomLoadedFlowNode.NavTarget.RoomDetails)!! assertThat(roomDetailsNode.id).isEqualTo(fakeRoomDetailsEntryPoint.nodeId) } + + @Test + fun `the ActiveRoomHolder will be updated with the loaded room on create`() = runTest { + // GIVEN + val room = FakeJoinedRoom(baseRoom = FakeBaseRoom(updateMembersResult = {})) + val fakeMessagesEntryPoint = FakeMessagesEntryPoint() + val fakeRoomDetailsEntryPoint = FakeRoomDetailsEntryPoint() + val inputs = JoinedRoomLoadedFlowNode.Inputs(room, RoomNavigationTarget.Messages()) + val activeRoomHolder = ActiveRoomHolder() + val roomFlowNode = createJoinedRoomLoadedFlowNode( + plugins = listOf(inputs), + messagesEntryPoint = fakeMessagesEntryPoint, + roomDetailsEntryPoint = fakeRoomDetailsEntryPoint, + coroutineScope = this, + activeRoomHolder = activeRoomHolder, + ) + + assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNull() + val roomFlowNodeTestHelper = roomFlowNode.parentNodeTestHelper() + // WHEN + roomFlowNodeTestHelper.assertChildHasLifecycle(JoinedRoomLoadedFlowNode.NavTarget.Messages(null), Lifecycle.State.CREATED) + // THEN + assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNotNull() + } + + @Test + fun `the ActiveRoomHolder will be removed on destroy`() = runTest { + // GIVEN + val room = FakeJoinedRoom(baseRoom = FakeBaseRoom(updateMembersResult = {})) + val fakeMessagesEntryPoint = FakeMessagesEntryPoint() + val fakeRoomDetailsEntryPoint = FakeRoomDetailsEntryPoint() + val inputs = JoinedRoomLoadedFlowNode.Inputs(room, RoomNavigationTarget.Messages()) + val activeRoomHolder = ActiveRoomHolder().apply { + addRoom(room) + } + val roomFlowNode = createJoinedRoomLoadedFlowNode( + plugins = listOf(inputs), + messagesEntryPoint = fakeMessagesEntryPoint, + roomDetailsEntryPoint = fakeRoomDetailsEntryPoint, + coroutineScope = this, + activeRoomHolder = activeRoomHolder, + ) + val roomFlowNodeTestHelper = roomFlowNode.parentNodeTestHelper() + roomFlowNodeTestHelper.assertChildHasLifecycle(JoinedRoomLoadedFlowNode.NavTarget.Messages(null), Lifecycle.State.CREATED) + assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNotNull() + // WHEN + roomFlowNode.updateLifecycleState(Lifecycle.State.DESTROYED) + // THEN + roomFlowNodeTestHelper.assertChildHasLifecycle(JoinedRoomLoadedFlowNode.NavTarget.Messages(null), Lifecycle.State.DESTROYED) + assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNull() + } } diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/ui/CallScreenPresenterTest.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/ui/CallScreenPresenterTest.kt index 43a1e7ceba5..e89538fbc30 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/ui/CallScreenPresenterTest.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/ui/CallScreenPresenterTest.kt @@ -32,6 +32,7 @@ import io.element.android.libraries.matrix.test.widget.FakeMatrixWidgetDriver import io.element.android.libraries.network.useragent.UserAgentProvider import io.element.android.services.analytics.api.ScreenTracker import io.element.android.services.analytics.test.FakeScreenTracker +import io.element.android.services.appnavstate.api.ActiveRoomHolder import io.element.android.services.appnavstate.test.FakeAppForegroundStateService import io.element.android.services.toolbox.api.systemclock.SystemClock import io.element.android.tests.testutils.WarmUpRule @@ -367,6 +368,7 @@ import kotlin.time.Duration.Companion.seconds activeCallManager: FakeActiveCallManager = FakeActiveCallManager(), screenTracker: ScreenTracker = FakeScreenTracker(), appForegroundStateService: FakeAppForegroundStateService = FakeAppForegroundStateService(), + activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), ): CallScreenPresenter { val userAgentProvider = object : UserAgentProvider { override fun provide(): String { @@ -387,6 +389,7 @@ import kotlin.time.Duration.Companion.seconds languageTagProvider = FakeLanguageTagProvider("en-US"), appForegroundStateService = appForegroundStateService, appCoroutineScope = backgroundScope, + activeRoomHolder = activeRoomHolder, ) } } diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultCallWidgetProviderTest.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultCallWidgetProviderTest.kt index cd81533a47d..ad68503a7ef 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultCallWidgetProviderTest.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultCallWidgetProviderTest.kt @@ -15,11 +15,13 @@ import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.matrix.test.A_SESSION_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.widget.FakeCallWidgetSettingsProvider import io.element.android.libraries.matrix.test.widget.FakeMatrixWidgetDriver import io.element.android.libraries.preferences.api.store.AppPreferencesStore import io.element.android.libraries.preferences.test.InMemoryAppPreferencesStore +import io.element.android.services.appnavstate.api.ActiveRoomHolder import kotlinx.coroutines.test.runTest import org.junit.Test @@ -77,6 +79,23 @@ class DefaultCallWidgetProviderTest { assertThat(provider.getWidget(A_SESSION_ID, A_ROOM_ID, "clientId", "languageTag", "theme").getOrNull()).isNotNull() } + @Test + fun `getWidget - reuses the active room if possible`() = runTest { + val client = FakeMatrixClient().apply { + // No room from the client + givenGetRoomResult(A_ROOM_ID, null) + } + val activeRoomHolder = ActiveRoomHolder().apply { + // A current active room with the same room id + addRoom(FakeJoinedRoom(baseRoom = FakeBaseRoom(roomId = A_ROOM_ID))) + } + val provider = createProvider( + matrixClientProvider = FakeMatrixClientProvider { Result.success(client) }, + activeRoomHolder = activeRoomHolder + ) + assertThat(provider.getWidget(A_SESSION_ID, A_ROOM_ID, "clientId", "languageTag", "theme").isFailure).isTrue() + } + @Test fun `getWidget - will use a custom base url if it exists`() = runTest { val room = FakeJoinedRoom( @@ -104,9 +123,11 @@ class DefaultCallWidgetProviderTest { matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider(), appPreferencesStore: AppPreferencesStore = InMemoryAppPreferencesStore(), callWidgetSettingsProvider: CallWidgetSettingsProvider = FakeCallWidgetSettingsProvider(), + activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), ) = DefaultCallWidgetProvider( matrixClientsProvider = matrixClientProvider, appPreferencesStore = appPreferencesStore, callWidgetSettingsProvider = callWidgetSettingsProvider, + activeRoomHolder = activeRoomHolder, ) } diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/tasks/DefaultClearCacheUseCaseTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/tasks/DefaultClearCacheUseCaseTest.kt index 401477d5fca..824d9584019 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/tasks/DefaultClearCacheUseCaseTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/tasks/DefaultClearCacheUseCaseTest.kt @@ -15,8 +15,11 @@ import io.element.android.features.invite.test.InMemorySeenInvitesStore import io.element.android.features.preferences.impl.DefaultCacheService import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.test.A_ROOM_ID +import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.FakeMatrixClient +import io.element.android.libraries.matrix.test.room.FakeJoinedRoom import io.element.android.libraries.push.test.FakePushService +import io.element.android.services.appnavstate.api.ActiveRoomHolder import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value import io.element.android.tests.testutils.testCoroutineDispatchers @@ -31,8 +34,10 @@ import org.robolectric.RobolectricTestRunner class DefaultClearCacheUseCaseTest { @Test fun `execute clear cache should do all the expected tasks`() = runTest { + val activeRoomHolder = ActiveRoomHolder().apply { addRoom(FakeJoinedRoom()) } val clearCacheLambda = lambdaRecorder { } val matrixClient = FakeMatrixClient( + sessionId = A_SESSION_ID, clearCacheLambda = clearCacheLambda, ) val defaultCacheService = DefaultCacheService() @@ -55,6 +60,7 @@ class DefaultClearCacheUseCaseTest { ftueService = ftueService, pushService = pushService, seenInvitesStore = seenInvitesStore, + activeRoomHolder = activeRoomHolder, ) defaultCacheService.clearedCacheEventFlow.test { sut.invoke() @@ -64,6 +70,7 @@ class DefaultClearCacheUseCaseTest { .with(value(matrixClient.sessionId), value(false)) assertThat(awaitItem()).isEqualTo(matrixClient.sessionId) assertThat(seenInvitesStore.seenRoomIds().first()).isEmpty() + assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNull() } } } diff --git a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt index 3504390f680..0045365910d 100644 --- a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt +++ b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt @@ -28,6 +28,7 @@ import io.element.android.libraries.matrix.test.timeline.FakeTimeline import io.element.android.libraries.mediaupload.api.MediaPreProcessor import io.element.android.libraries.mediaupload.test.FakeMediaPreProcessor import io.element.android.libraries.preferences.test.InMemorySessionPreferencesStore +import io.element.android.services.appnavstate.api.ActiveRoomHolder import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.lambda.lambdaRecorder import kotlinx.coroutines.test.TestScope @@ -163,7 +164,8 @@ class SharePresenterTest { intent: Intent = Intent(), shareIntentHandler: ShareIntentHandler = FakeShareIntentHandler(), matrixClient: MatrixClient = FakeMatrixClient(), - mediaPreProcessor: MediaPreProcessor = FakeMediaPreProcessor() + mediaPreProcessor: MediaPreProcessor = FakeMediaPreProcessor(), + activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), ): SharePresenter { return SharePresenter( intent = intent, @@ -171,7 +173,8 @@ class SharePresenterTest { shareIntentHandler = shareIntentHandler, matrixClient = matrixClient, mediaPreProcessor = mediaPreProcessor, - InMemorySessionPreferencesStore(), + sessionPreferencesStore = InMemorySessionPreferencesStore(), + activeRoomHolder = activeRoomHolder, ) } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt index 8a4f5352163..1dbeda65d61 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt @@ -41,6 +41,7 @@ import io.element.android.libraries.push.impl.notifications.model.NotifiableEven import io.element.android.libraries.push.impl.push.FakeOnNotifiableEventReceived import io.element.android.libraries.push.impl.push.OnNotifiableEventReceived import io.element.android.libraries.push.test.notifications.FakeNotificationCleaner +import io.element.android.services.appnavstate.api.ActiveRoomHolder import io.element.android.services.toolbox.api.strings.StringProvider import io.element.android.services.toolbox.api.systemclock.SystemClock import io.element.android.services.toolbox.test.strings.FakeStringProvider @@ -477,6 +478,7 @@ class NotificationBroadcastReceiverHandlerTest { onNotifiableEventReceived: OnNotifiableEventReceived = FakeOnNotifiableEventReceived(), stringProvider: StringProvider = FakeStringProvider(), replyMessageExtractor: ReplyMessageExtractor = FakeReplyMessageExtractor(), + activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), ): NotificationBroadcastReceiverHandler { return NotificationBroadcastReceiverHandler( appCoroutineScope = this, @@ -494,6 +496,7 @@ class NotificationBroadcastReceiverHandlerTest { onNotifiableEventReceived = onNotifiableEventReceived, stringProvider = stringProvider, replyMessageExtractor = replyMessageExtractor, + activeRoomHolder = activeRoomHolder, ) } } 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 8b3907be0b4..0b7bf135d52 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 @@ -26,6 +26,7 @@ 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.ActiveRoomHolder import io.element.android.services.appnavstate.test.FakeAppForegroundStateService import io.element.android.tests.testutils.lambda.assert import io.element.android.tests.testutils.lambda.lambdaRecorder @@ -199,7 +200,8 @@ class SyncOnNotifiableEventTest { isSyncOnPushEnabled: Boolean = true, appForegroundStateService: FakeAppForegroundStateService = FakeAppForegroundStateService( initialForegroundValue = true, - ) + ), + activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), ): SyncOnNotifiableEvent { val featureFlagService = FakeFeatureFlagService( initialState = mapOf( @@ -212,6 +214,7 @@ class SyncOnNotifiableEventTest { featureFlagService = featureFlagService, appForegroundStateService = appForegroundStateService, dispatchers = testCoroutineDispatchers(), + activeRoomHolder = activeRoomHolder, ) } } From 0f3d2c40b5556e93a9c0909c7e876c397ae6275f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 22 May 2025 14:19:50 +0200 Subject: [PATCH 3/9] Try fixing the review comments --- .../call/impl/ui/CallScreenPresenter.kt | 9 ++-- .../impl/utils/DefaultCallWidgetProvider.kt | 2 +- .../NotificationBroadcastReceiverHandler.kt | 4 +- .../push/impl/push/SyncOnNotifiableEvent.kt | 44 ++++++++++++------- .../appnavstate/api/ActiveRoomHolder.kt | 34 +++++++++++--- 5 files changed, 62 insertions(+), 31 deletions(-) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt index 84dfc4767bc..b217d7e2754 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt @@ -243,12 +243,9 @@ class CallScreenPresenter @AssistedInject constructor( private suspend fun MatrixClient.notifyCallStartIfNeeded(roomId: RoomId) { if (!notifiedCallStart) { - val activeRoomForSession = activeRoomHolder.getActiveRoom(sessionId) - val sendCallNotificationResult = if (activeRoomForSession?.roomId == roomId) { - activeRoomForSession.sendCallNotificationIfNeeded() - } else { - getJoinedRoom(roomId)?.use { it.sendCallNotificationIfNeeded() } - } + val activeRoomForSession = activeRoomHolder.getActiveRoomMatching(sessionId, roomId) + val sendCallNotificationResult = activeRoomForSession?.sendCallNotificationIfNeeded() + ?: getJoinedRoom(roomId)?.use { it.sendCallNotificationIfNeeded() } sendCallNotificationResult?.onSuccess { notifiedCallStart = true } } } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt index 9e418bfef92..cfc2a017b3d 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt @@ -35,7 +35,7 @@ class DefaultCallWidgetProvider @Inject constructor( theme: String?, ): Result = runCatching { val matrixClient = matrixClientsProvider.getOrRestore(sessionId).getOrThrow() - val room = activeRoomHolder.getActiveRoom(sessionId)?.takeIf { it.roomId == roomId } + val room = activeRoomHolder.getActiveRoomMatching(sessionId, roomId) ?: matrixClient.getJoinedRoom(roomId) ?: error("Room not found") diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt index aa228d0dec8..21139279c78 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt @@ -119,9 +119,7 @@ class NotificationBroadcastReceiverHandler @Inject constructor( return@launch } val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: return@launch - val room = activeRoomHolder.getActiveRoom(sessionId) - ?.takeIf { it.roomId == roomId } - ?: client.getJoinedRoom(roomId) + val room = activeRoomHolder.getActiveRoomMatching(sessionId, roomId) ?: client.getJoinedRoom(roomId) room?.let { sendMatrixEvent( 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 a9e8aa2762e..6a7acd97ad0 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 @@ -39,23 +39,35 @@ class SyncOnNotifiableEvent @Inject constructor( return@withContext } - val notifiedRoom = activeRoomHolder.getActiveRoom(sessionId = notifiableEvent.sessionId) - ?: matrixClientProvider.getOrRestore(notifiableEvent.sessionId).getOrNull()?.getJoinedRoom(notifiableEvent.roomId) - ?: return@withContext - notifiedRoom.use { room -> - room.subscribeToSync() + val activeRoom = activeRoomHolder.getActiveRoomMatching(notifiableEvent.sessionId, notifiableEvent.roomId) - // If the app is in foreground, sync is already running, so we just add the subscription above. - if (!appForegroundStateService.isInForeground.value) { - if (isRingingCallEvent) { - room.waitsUntilUserIsInTheCall(timeout = 60.seconds) - } else { - try { - appForegroundStateService.updateIsSyncingNotificationEvent(true) - room.waitsUntilEventIsKnown(eventId = notifiableEvent.eventId, timeout = 10.seconds) - } finally { - appForegroundStateService.updateIsSyncingNotificationEvent(false) - } + if (activeRoom != null) { + // If the room is already active, we can use it directly + activeRoom.subscribeToSyncAndWait(notifiableEvent, isRingingCallEvent) + } else { + // Otherwise, we need to get the room from the matrix client + val room = matrixClientProvider + .getOrRestore(notifiableEvent.sessionId) + .mapCatching { it.getJoinedRoom(notifiableEvent.roomId) } + .getOrNull() + + room?.use { it.subscribeToSyncAndWait(notifiableEvent, isRingingCallEvent) } + } + } + + private suspend fun JoinedRoom.subscribeToSyncAndWait(notifiableEvent: NotifiableEvent, isRingingCallEvent: Boolean) { + subscribeToSync() + + // If the app is in foreground, sync is already running, so we just add the subscription above. + if (!appForegroundStateService.isInForeground.value) { + if (isRingingCallEvent) { + waitsUntilUserIsInTheCall(timeout = 60.seconds) + } else { + try { + appForegroundStateService.updateIsSyncingNotificationEvent(true) + waitsUntilEventIsKnown(eventId = notifiableEvent.eventId, timeout = 10.seconds) + } finally { + appForegroundStateService.updateIsSyncingNotificationEvent(false) } } } diff --git a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt index 837500bd23f..de47dcfe42b 100644 --- a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt +++ b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt @@ -9,27 +9,51 @@ package io.element.android.services.appnavstate.api import io.element.android.libraries.di.AppScope import io.element.android.libraries.di.SingleIn +import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.room.JoinedRoom import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject /** - * Holds the active room for a given session so it can be reused instead of instantiating a new one. + * Holds the active rooms for a given session so they can be reused instead of instantiating new ones. + * + * This works as a FILO (First In Last Out) stack, meaning that the last room added for a session will be the first one to be removed. */ @SingleIn(AppScope::class) class ActiveRoomHolder @Inject constructor() { - private val rooms = ConcurrentHashMap() + private val rooms = ConcurrentHashMap>() + /** + * Adds a new held room for the given sessionId. + */ fun addRoom(room: JoinedRoom) { - rooms[room.sessionId] = room + val roomsForSessionId = rooms.getOrPut(key = room.sessionId, defaultValue = { mutableListOf() }) + if (roomsForSessionId.none { it.roomId == room.roomId }) { + // We don't want to add the same room multiple times + roomsForSessionId.add(room) + } } + /** + * Returns the last room added for the given [sessionId] or null if no room was added. + */ fun getActiveRoom(sessionId: SessionId): JoinedRoom? { - return rooms[sessionId] + return rooms[sessionId]?.lastOrNull() + } + + /** + * Returns an active room associated to the given [sessionId], with the given [roomId], or null if none match. + */ + fun getActiveRoomMatching(sessionId: SessionId, roomId: RoomId): JoinedRoom? { + return rooms[sessionId]?.find { it.roomId == roomId } } + /** + * Removes the last room added for the given [sessionId] and returns it or null if there weren't any. + */ fun removeRoom(sessionId: SessionId): JoinedRoom? { - return rooms.remove(sessionId) + val roomsForSessionId = rooms[sessionId] ?: return null + return roomsForSessionId.removeLastOrNull() } } From 9b5fa7765fba136fb33d22f63e814ea9a450530a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 22 May 2025 15:58:07 +0200 Subject: [PATCH 4/9] Fix ending a call destroying the underlying room, since this might be held by the `ActiveRoomHolder` --- .../android/libraries/matrix/impl/room/JoinedRustRoom.kt | 1 - .../android/libraries/matrix/impl/widget/RustWidgetDriver.kt | 2 -- 2 files changed, 3 deletions(-) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/JoinedRustRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/JoinedRustRoom.kt index d01675bb295..79e3a1e0d23 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/JoinedRustRoom.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/JoinedRustRoom.kt @@ -416,7 +416,6 @@ class JoinedRustRoom( RustWidgetDriver( widgetSettings = widgetSettings, room = innerRoom, - joinedRustRoom = this, widgetCapabilitiesProvider = object : WidgetCapabilitiesProvider { override fun acquireCapabilities(capabilities: WidgetCapabilities): WidgetCapabilities { return getElementCallRequiredPermissions(sessionId.value, baseRoom.deviceId.value) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/widget/RustWidgetDriver.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/widget/RustWidgetDriver.kt index b723ab194ba..f736ef2e3bb 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/widget/RustWidgetDriver.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/widget/RustWidgetDriver.kt @@ -25,7 +25,6 @@ import kotlin.coroutines.coroutineContext class RustWidgetDriver( widgetSettings: MatrixWidgetSettings, private val room: Room, - private val joinedRustRoom: JoinedRustRoom, private val widgetCapabilitiesProvider: WidgetCapabilitiesProvider, ) : MatrixWidgetDriver { // It's important to have extra capacity here to make sure we don't drop any messages @@ -71,6 +70,5 @@ class RustWidgetDriver( override fun close() { receiveMessageJob?.cancel() driverAndHandle.driver.close() - joinedRustRoom.destroy() } } From f7549de83b94ed1f4bb0843097c035b294d6255a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 22 May 2025 16:01:13 +0200 Subject: [PATCH 5/9] Also fix share presenter closing the room if it's held --- .../element/android/features/share/impl/SharePresenter.kt | 6 +++++- .../libraries/matrix/impl/widget/RustWidgetDriver.kt | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt index 4b87163255d..eadeebc9df4 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt @@ -95,7 +95,11 @@ class SharePresenter @AssistedInject constructor( ).isSuccess } .all { it } - .also { room.destroy() } + .also { + if (activeRoomHolder.getActiveRoomMatching(matrixClient.sessionId, roomId) == null) { + room.destroy() + } + } } .all { it } } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/widget/RustWidgetDriver.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/widget/RustWidgetDriver.kt index f736ef2e3bb..c80313f5da6 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/widget/RustWidgetDriver.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/widget/RustWidgetDriver.kt @@ -9,7 +9,6 @@ package io.element.android.libraries.matrix.impl.widget import io.element.android.libraries.matrix.api.widget.MatrixWidgetDriver import io.element.android.libraries.matrix.api.widget.MatrixWidgetSettings -import io.element.android.libraries.matrix.impl.room.JoinedRustRoom import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job From 51ec8596544ba05b2762a1bce49a98641ccf57b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 23 May 2025 16:01:03 +0200 Subject: [PATCH 6/9] Rename `ActiveRoomHolder` to `ActiveRoomsHolder` --- .../room/joined/JoinedRoomLoadedFlowNode.kt | 7 ++--- .../appnav/JoinedRoomLoadedFlowNodeTest.kt | 26 +++++++++---------- .../call/impl/ui/CallScreenPresenter.kt | 6 ++--- .../impl/utils/DefaultCallWidgetProvider.kt | 6 ++--- .../call/ui/CallScreenPresenterTest.kt | 6 ++--- .../utils/DefaultCallWidgetProviderTest.kt | 10 +++---- .../impl/tasks/ClearCacheUseCase.kt | 5 ++-- .../tasks/DefaultClearCacheUseCaseTest.kt | 8 +++--- .../features/share/impl/SharePresenter.kt | 8 +++--- .../features/share/impl/SharePresenterTest.kt | 6 ++--- .../NotificationBroadcastReceiverHandler.kt | 6 ++--- .../push/impl/push/SyncOnNotifiableEvent.kt | 6 ++--- ...otificationBroadcastReceiverHandlerTest.kt | 6 ++--- .../impl/push/SyncOnNotifiableEventTest.kt | 6 ++--- ...tiveRoomHolder.kt => ActiveRoomsHolder.kt} | 2 +- 15 files changed, 58 insertions(+), 56 deletions(-) rename services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/{ActiveRoomHolder.kt => ActiveRoomsHolder.kt} (97%) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt index 68c289d11b0..ef308eac3aa 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt @@ -36,7 +36,7 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.permalink.PermalinkData import io.element.android.libraries.matrix.api.room.JoinedRoom -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.services.appnavstate.api.AppNavigationStateService import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -52,7 +52,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor( private val appNavigationStateService: AppNavigationStateService, private val appCoroutineScope: CoroutineScope, private val matrixClient: MatrixClient, - private val activeRoomHolder: ActiveRoomHolder, + private val activeRoomsHolder: ActiveRoomsHolder, roomComponentFactory: RoomComponentFactory, ) : BaseFlowNode( backstack = BackStack( @@ -87,7 +87,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor( onCreate = { Timber.v("OnCreate => ${inputs.room.roomId}") appNavigationStateService.onNavigateToRoom(id, inputs.room.roomId) - activeRoomHolder.addRoom(inputs.room) + activeRoomsHolder.addRoom(inputs.room) fetchRoomMembers() trackVisitedRoom() }, @@ -99,6 +99,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor( onDestroy = { Timber.v("OnDestroy") activeRoomHolder.removeRoom(inputs.room.sessionId) + activeRoomsHolder.removeRoom(inputs.room.sessionId) inputs.room.destroy() appNavigationStateService.onLeavingRoom(id) } diff --git a/appnav/src/test/kotlin/io/element/android/appnav/JoinedRoomLoadedFlowNodeTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/JoinedRoomLoadedFlowNodeTest.kt index 7b61d677fb0..f8d4be1807b 100644 --- a/appnav/src/test/kotlin/io/element/android/appnav/JoinedRoomLoadedFlowNodeTest.kt +++ b/appnav/src/test/kotlin/io/element/android/appnav/JoinedRoomLoadedFlowNodeTest.kt @@ -28,7 +28,7 @@ import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.room.FakeBaseRoom import io.element.android.libraries.matrix.test.room.FakeJoinedRoom -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.services.appnavstate.test.FakeAppNavigationStateService import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.test.runTest @@ -102,7 +102,7 @@ class JoinedRoomLoadedFlowNodeTest { plugins: List, messagesEntryPoint: MessagesEntryPoint = FakeMessagesEntryPoint(), roomDetailsEntryPoint: RoomDetailsEntryPoint = FakeRoomDetailsEntryPoint(), - activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), + activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(), coroutineScope: CoroutineScope, ) = JoinedRoomLoadedFlowNode( buildContext = BuildContext.root(savedStateMap = null), @@ -113,7 +113,7 @@ class JoinedRoomLoadedFlowNodeTest { appCoroutineScope = coroutineScope, roomComponentFactory = FakeRoomComponentFactory(), matrixClient = FakeMatrixClient(), - activeRoomHolder = activeRoomHolder, + activeRoomsHolder = activeRoomsHolder, ) @Test @@ -160,37 +160,37 @@ class JoinedRoomLoadedFlowNodeTest { } @Test - fun `the ActiveRoomHolder will be updated with the loaded room on create`() = runTest { + fun `the ActiveRoomsHolder will be updated with the loaded room on create`() = runTest { // GIVEN val room = FakeJoinedRoom(baseRoom = FakeBaseRoom(updateMembersResult = {})) val fakeMessagesEntryPoint = FakeMessagesEntryPoint() val fakeRoomDetailsEntryPoint = FakeRoomDetailsEntryPoint() val inputs = JoinedRoomLoadedFlowNode.Inputs(room, RoomNavigationTarget.Messages()) - val activeRoomHolder = ActiveRoomHolder() + val activeRoomsHolder = ActiveRoomsHolder() val roomFlowNode = createJoinedRoomLoadedFlowNode( plugins = listOf(inputs), messagesEntryPoint = fakeMessagesEntryPoint, roomDetailsEntryPoint = fakeRoomDetailsEntryPoint, coroutineScope = this, - activeRoomHolder = activeRoomHolder, + activeRoomsHolder = activeRoomsHolder, ) - assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNull() + assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNull() val roomFlowNodeTestHelper = roomFlowNode.parentNodeTestHelper() // WHEN roomFlowNodeTestHelper.assertChildHasLifecycle(JoinedRoomLoadedFlowNode.NavTarget.Messages(null), Lifecycle.State.CREATED) // THEN - assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNotNull() + assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNotNull() } @Test - fun `the ActiveRoomHolder will be removed on destroy`() = runTest { + fun `the ActiveRoomsHolder will be removed on destroy`() = runTest { // GIVEN val room = FakeJoinedRoom(baseRoom = FakeBaseRoom(updateMembersResult = {})) val fakeMessagesEntryPoint = FakeMessagesEntryPoint() val fakeRoomDetailsEntryPoint = FakeRoomDetailsEntryPoint() val inputs = JoinedRoomLoadedFlowNode.Inputs(room, RoomNavigationTarget.Messages()) - val activeRoomHolder = ActiveRoomHolder().apply { + val activeRoomsHolder = ActiveRoomsHolder().apply { addRoom(room) } val roomFlowNode = createJoinedRoomLoadedFlowNode( @@ -198,15 +198,15 @@ class JoinedRoomLoadedFlowNodeTest { messagesEntryPoint = fakeMessagesEntryPoint, roomDetailsEntryPoint = fakeRoomDetailsEntryPoint, coroutineScope = this, - activeRoomHolder = activeRoomHolder, + activeRoomsHolder = activeRoomsHolder, ) val roomFlowNodeTestHelper = roomFlowNode.parentNodeTestHelper() roomFlowNodeTestHelper.assertChildHasLifecycle(JoinedRoomLoadedFlowNode.NavTarget.Messages(null), Lifecycle.State.CREATED) - assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNotNull() + assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNotNull() // WHEN roomFlowNode.updateLifecycleState(Lifecycle.State.DESTROYED) // THEN roomFlowNodeTestHelper.assertChildHasLifecycle(JoinedRoomLoadedFlowNode.NavTarget.Messages(null), Lifecycle.State.DESTROYED) - assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNull() + assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNull() } } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt index b217d7e2754..8550f06f3d2 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt @@ -39,7 +39,7 @@ import io.element.android.libraries.matrix.api.sync.SyncState import io.element.android.libraries.matrix.api.widget.MatrixWidgetDriver import io.element.android.libraries.network.useragent.UserAgentProvider import io.element.android.services.analytics.api.ScreenTracker -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.services.appnavstate.api.AppForegroundStateService import io.element.android.services.toolbox.api.systemclock.SystemClock import kotlinx.coroutines.CoroutineScope @@ -63,7 +63,7 @@ class CallScreenPresenter @AssistedInject constructor( private val activeCallManager: ActiveCallManager, private val languageTagProvider: LanguageTagProvider, private val appForegroundStateService: AppForegroundStateService, - private val activeRoomHolder: ActiveRoomHolder, + private val activeRoomsHolder: ActiveRoomsHolder, private val appCoroutineScope: CoroutineScope, ) : Presenter { @AssistedFactory @@ -243,7 +243,7 @@ class CallScreenPresenter @AssistedInject constructor( private suspend fun MatrixClient.notifyCallStartIfNeeded(roomId: RoomId) { if (!notifiedCallStart) { - val activeRoomForSession = activeRoomHolder.getActiveRoomMatching(sessionId, roomId) + val activeRoomForSession = activeRoomsHolder.getActiveRoomMatching(sessionId, roomId) val sendCallNotificationResult = activeRoomForSession?.sendCallNotificationIfNeeded() ?: getJoinedRoom(roomId)?.use { it.sendCallNotificationIfNeeded() } sendCallNotificationResult?.onSuccess { notifiedCallStart = true } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt index cfc2a017b3d..dd4de7abb53 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/DefaultCallWidgetProvider.kt @@ -14,7 +14,7 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.widget.CallWidgetSettingsProvider import io.element.android.libraries.preferences.api.store.AppPreferencesStore -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import kotlinx.coroutines.flow.firstOrNull import javax.inject.Inject @@ -25,7 +25,7 @@ class DefaultCallWidgetProvider @Inject constructor( private val matrixClientsProvider: MatrixClientProvider, private val appPreferencesStore: AppPreferencesStore, private val callWidgetSettingsProvider: CallWidgetSettingsProvider, - private val activeRoomHolder: ActiveRoomHolder, + private val activeRoomsHolder: ActiveRoomsHolder, ) : CallWidgetProvider { override suspend fun getWidget( sessionId: SessionId, @@ -35,7 +35,7 @@ class DefaultCallWidgetProvider @Inject constructor( theme: String?, ): Result = runCatching { val matrixClient = matrixClientsProvider.getOrRestore(sessionId).getOrThrow() - val room = activeRoomHolder.getActiveRoomMatching(sessionId, roomId) + val room = activeRoomsHolder.getActiveRoomMatching(sessionId, roomId) ?: matrixClient.getJoinedRoom(roomId) ?: error("Room not found") diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/ui/CallScreenPresenterTest.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/ui/CallScreenPresenterTest.kt index e89538fbc30..ef571e12b30 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/ui/CallScreenPresenterTest.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/ui/CallScreenPresenterTest.kt @@ -32,7 +32,7 @@ import io.element.android.libraries.matrix.test.widget.FakeMatrixWidgetDriver import io.element.android.libraries.network.useragent.UserAgentProvider import io.element.android.services.analytics.api.ScreenTracker import io.element.android.services.analytics.test.FakeScreenTracker -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.services.appnavstate.test.FakeAppForegroundStateService import io.element.android.services.toolbox.api.systemclock.SystemClock import io.element.android.tests.testutils.WarmUpRule @@ -368,7 +368,7 @@ import kotlin.time.Duration.Companion.seconds activeCallManager: FakeActiveCallManager = FakeActiveCallManager(), screenTracker: ScreenTracker = FakeScreenTracker(), appForegroundStateService: FakeAppForegroundStateService = FakeAppForegroundStateService(), - activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), + activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(), ): CallScreenPresenter { val userAgentProvider = object : UserAgentProvider { override fun provide(): String { @@ -389,7 +389,7 @@ import kotlin.time.Duration.Companion.seconds languageTagProvider = FakeLanguageTagProvider("en-US"), appForegroundStateService = appForegroundStateService, appCoroutineScope = backgroundScope, - activeRoomHolder = activeRoomHolder, + activeRoomsHolder = activeRoomsHolder, ) } } diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultCallWidgetProviderTest.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultCallWidgetProviderTest.kt index ad68503a7ef..f9267eca7bb 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultCallWidgetProviderTest.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultCallWidgetProviderTest.kt @@ -21,7 +21,7 @@ import io.element.android.libraries.matrix.test.widget.FakeCallWidgetSettingsPro import io.element.android.libraries.matrix.test.widget.FakeMatrixWidgetDriver import io.element.android.libraries.preferences.api.store.AppPreferencesStore import io.element.android.libraries.preferences.test.InMemoryAppPreferencesStore -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import kotlinx.coroutines.test.runTest import org.junit.Test @@ -85,13 +85,13 @@ class DefaultCallWidgetProviderTest { // No room from the client givenGetRoomResult(A_ROOM_ID, null) } - val activeRoomHolder = ActiveRoomHolder().apply { + val activeRoomsHolder = ActiveRoomsHolder().apply { // A current active room with the same room id addRoom(FakeJoinedRoom(baseRoom = FakeBaseRoom(roomId = A_ROOM_ID))) } val provider = createProvider( matrixClientProvider = FakeMatrixClientProvider { Result.success(client) }, - activeRoomHolder = activeRoomHolder + activeRoomsHolder = activeRoomsHolder ) assertThat(provider.getWidget(A_SESSION_ID, A_ROOM_ID, "clientId", "languageTag", "theme").isFailure).isTrue() } @@ -123,11 +123,11 @@ class DefaultCallWidgetProviderTest { matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider(), appPreferencesStore: AppPreferencesStore = InMemoryAppPreferencesStore(), callWidgetSettingsProvider: CallWidgetSettingsProvider = FakeCallWidgetSettingsProvider(), - activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), + activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(), ) = DefaultCallWidgetProvider( matrixClientsProvider = matrixClientProvider, appPreferencesStore = appPreferencesStore, callWidgetSettingsProvider = callWidgetSettingsProvider, - activeRoomHolder = activeRoomHolder, + activeRoomsHolder = activeRoomsHolder, ) } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt index 3a252008f1f..494c25bff35 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt @@ -18,7 +18,7 @@ import io.element.android.libraries.di.ApplicationContext import io.element.android.libraries.di.SessionScope import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.push.api.PushService -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import kotlinx.coroutines.withContext import okhttp3.OkHttpClient import javax.inject.Inject @@ -38,11 +38,12 @@ class DefaultClearCacheUseCase @Inject constructor( private val ftueService: FtueService, private val pushService: PushService, private val seenInvitesStore: SeenInvitesStore, - private val activeRoomHolder: ActiveRoomHolder, + private val activeRoomsHolder: ActiveRoomsHolder, ) : ClearCacheUseCase { override suspend fun invoke() = withContext(coroutineDispatchers.io) { // Active rooms should be disposed of before clearing the cache activeRoomHolder.removeRoom(matrixClient.sessionId)?.destroy() + activeRoomsHolder.removeRoom(matrixClient.sessionId) // Clear Matrix cache matrixClient.clearCache() // Clear Coil cache diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/tasks/DefaultClearCacheUseCaseTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/tasks/DefaultClearCacheUseCaseTest.kt index 824d9584019..ab391bfc625 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/tasks/DefaultClearCacheUseCaseTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/tasks/DefaultClearCacheUseCaseTest.kt @@ -19,7 +19,7 @@ import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.room.FakeJoinedRoom import io.element.android.libraries.push.test.FakePushService -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value import io.element.android.tests.testutils.testCoroutineDispatchers @@ -34,7 +34,7 @@ import org.robolectric.RobolectricTestRunner class DefaultClearCacheUseCaseTest { @Test fun `execute clear cache should do all the expected tasks`() = runTest { - val activeRoomHolder = ActiveRoomHolder().apply { addRoom(FakeJoinedRoom()) } + val activeRoomsHolder = ActiveRoomsHolder().apply { addRoom(FakeJoinedRoom()) } val clearCacheLambda = lambdaRecorder { } val matrixClient = FakeMatrixClient( sessionId = A_SESSION_ID, @@ -60,7 +60,7 @@ class DefaultClearCacheUseCaseTest { ftueService = ftueService, pushService = pushService, seenInvitesStore = seenInvitesStore, - activeRoomHolder = activeRoomHolder, + activeRoomsHolder = activeRoomsHolder, ) defaultCacheService.clearedCacheEventFlow.test { sut.invoke() @@ -70,7 +70,7 @@ class DefaultClearCacheUseCaseTest { .with(value(matrixClient.sessionId), value(false)) assertThat(awaitItem()).isEqualTo(matrixClient.sessionId) assertThat(seenInvitesStore.seenRoomIds().first()).isEmpty() - assertThat(activeRoomHolder.getActiveRoom(A_SESSION_ID)).isNull() + assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNull() } } } diff --git a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt index eadeebc9df4..1c75daa828e 100644 --- a/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt +++ b/features/share/impl/src/main/kotlin/io/element/android/features/share/impl/SharePresenter.kt @@ -24,7 +24,7 @@ import io.element.android.libraries.matrix.api.room.JoinedRoom import io.element.android.libraries.mediaupload.api.MediaPreProcessor import io.element.android.libraries.mediaupload.api.MediaSender import io.element.android.libraries.preferences.api.store.SessionPreferencesStore -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -35,7 +35,7 @@ class SharePresenter @AssistedInject constructor( private val matrixClient: MatrixClient, private val mediaPreProcessor: MediaPreProcessor, private val sessionPreferencesStore: SessionPreferencesStore, - private val activeRoomHolder: ActiveRoomHolder, + private val activeRoomsHolder: ActiveRoomsHolder, ) : Presenter { @AssistedFactory interface Factory { @@ -63,7 +63,7 @@ class SharePresenter @AssistedInject constructor( } private suspend fun getJoinedRoom(roomId: RoomId): JoinedRoom? { - return activeRoomHolder.getActiveRoom(matrixClient.sessionId) + return activeRoomsHolder.getActiveRoom(matrixClient.sessionId) ?.takeIf { it.roomId == roomId } ?: matrixClient.getJoinedRoom(roomId) } @@ -96,7 +96,7 @@ class SharePresenter @AssistedInject constructor( } .all { it } .also { - if (activeRoomHolder.getActiveRoomMatching(matrixClient.sessionId, roomId) == null) { + if (activeRoomsHolder.getActiveRoomMatching(matrixClient.sessionId, roomId) == null) { room.destroy() } } diff --git a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt index 0045365910d..07424aa384d 100644 --- a/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt +++ b/features/share/impl/src/test/kotlin/io/element/android/features/share/impl/SharePresenterTest.kt @@ -28,7 +28,7 @@ import io.element.android.libraries.matrix.test.timeline.FakeTimeline import io.element.android.libraries.mediaupload.api.MediaPreProcessor import io.element.android.libraries.mediaupload.test.FakeMediaPreProcessor import io.element.android.libraries.preferences.test.InMemorySessionPreferencesStore -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.lambda.lambdaRecorder import kotlinx.coroutines.test.TestScope @@ -165,7 +165,7 @@ class SharePresenterTest { shareIntentHandler: ShareIntentHandler = FakeShareIntentHandler(), matrixClient: MatrixClient = FakeMatrixClient(), mediaPreProcessor: MediaPreProcessor = FakeMediaPreProcessor(), - activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), + activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(), ): SharePresenter { return SharePresenter( intent = intent, @@ -174,7 +174,7 @@ class SharePresenterTest { matrixClient = matrixClient, mediaPreProcessor = mediaPreProcessor, sessionPreferencesStore = InMemorySessionPreferencesStore(), - activeRoomHolder = activeRoomHolder, + activeRoomsHolder = activeRoomsHolder, ) } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt index 21139279c78..f597fc7e393 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt @@ -23,7 +23,7 @@ import io.element.android.libraries.push.api.notifications.NotificationCleaner import io.element.android.libraries.push.impl.R import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent import io.element.android.libraries.push.impl.push.OnNotifiableEventReceived -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.services.toolbox.api.strings.StringProvider import io.element.android.services.toolbox.api.systemclock.SystemClock import kotlinx.coroutines.CoroutineScope @@ -45,7 +45,7 @@ class NotificationBroadcastReceiverHandler @Inject constructor( private val onNotifiableEventReceived: OnNotifiableEventReceived, private val stringProvider: StringProvider, private val replyMessageExtractor: ReplyMessageExtractor, - private val activeRoomHolder: ActiveRoomHolder, + private val activeRoomsHolder: ActiveRoomsHolder, ) { fun onReceive(intent: Intent) { val sessionId = intent.getStringExtra(NotificationBroadcastReceiver.KEY_SESSION_ID)?.let(::SessionId) ?: return @@ -119,7 +119,7 @@ class NotificationBroadcastReceiverHandler @Inject constructor( return@launch } val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: return@launch - val room = activeRoomHolder.getActiveRoomMatching(sessionId, roomId) ?: client.getJoinedRoom(roomId) + val room = activeRoomsHolder.getActiveRoomMatching(sessionId, roomId) ?: client.getJoinedRoom(roomId) room?.let { sendMatrixEvent( 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 6a7acd97ad0..23cad2aba95 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 @@ -17,7 +17,7 @@ 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.libraries.push.impl.notifications.model.NotifiableRingingCallEvent -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.services.appnavstate.api.AppForegroundStateService import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext @@ -31,7 +31,7 @@ class SyncOnNotifiableEvent @Inject constructor( private val featureFlagService: FeatureFlagService, private val appForegroundStateService: AppForegroundStateService, private val dispatchers: CoroutineDispatchers, - private val activeRoomHolder: ActiveRoomHolder, + private val activeRoomsHolder: ActiveRoomsHolder, ) { suspend operator fun invoke(notifiableEvent: NotifiableEvent) = withContext(dispatchers.io) { val isRingingCallEvent = notifiableEvent is NotifiableRingingCallEvent @@ -39,7 +39,7 @@ class SyncOnNotifiableEvent @Inject constructor( return@withContext } - val activeRoom = activeRoomHolder.getActiveRoomMatching(notifiableEvent.sessionId, notifiableEvent.roomId) + val activeRoom = activeRoomsHolder.getActiveRoomMatching(notifiableEvent.sessionId, notifiableEvent.roomId) if (activeRoom != null) { // If the room is already active, we can use it directly diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt index 1dbeda65d61..f86bd5fa701 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt @@ -41,7 +41,7 @@ import io.element.android.libraries.push.impl.notifications.model.NotifiableEven import io.element.android.libraries.push.impl.push.FakeOnNotifiableEventReceived import io.element.android.libraries.push.impl.push.OnNotifiableEventReceived import io.element.android.libraries.push.test.notifications.FakeNotificationCleaner -import io.element.android.services.appnavstate.api.ActiveRoomHolder +import io.element.android.services.appnavstate.api.ActiveRoomsHolder import io.element.android.services.toolbox.api.strings.StringProvider import io.element.android.services.toolbox.api.systemclock.SystemClock import io.element.android.services.toolbox.test.strings.FakeStringProvider @@ -478,7 +478,7 @@ class NotificationBroadcastReceiverHandlerTest { onNotifiableEventReceived: OnNotifiableEventReceived = FakeOnNotifiableEventReceived(), stringProvider: StringProvider = FakeStringProvider(), replyMessageExtractor: ReplyMessageExtractor = FakeReplyMessageExtractor(), - activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), + activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(), ): NotificationBroadcastReceiverHandler { return NotificationBroadcastReceiverHandler( appCoroutineScope = this, @@ -496,7 +496,7 @@ class NotificationBroadcastReceiverHandlerTest { onNotifiableEventReceived = onNotifiableEventReceived, stringProvider = stringProvider, replyMessageExtractor = replyMessageExtractor, - activeRoomHolder = activeRoomHolder, + activeRoomsHolder = activeRoomsHolder, ) } } 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 0b7bf135d52..9209eb4aebc 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 @@ -26,7 +26,7 @@ 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.ActiveRoomHolder +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 @@ -201,7 +201,7 @@ class SyncOnNotifiableEventTest { appForegroundStateService: FakeAppForegroundStateService = FakeAppForegroundStateService( initialForegroundValue = true, ), - activeRoomHolder: ActiveRoomHolder = ActiveRoomHolder(), + activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(), ): SyncOnNotifiableEvent { val featureFlagService = FakeFeatureFlagService( initialState = mapOf( @@ -214,7 +214,7 @@ class SyncOnNotifiableEventTest { featureFlagService = featureFlagService, appForegroundStateService = appForegroundStateService, dispatchers = testCoroutineDispatchers(), - activeRoomHolder = activeRoomHolder, + activeRoomsHolder = activeRoomsHolder, ) } } diff --git a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt similarity index 97% rename from services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt rename to services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt index de47dcfe42b..613c2191484 100644 --- a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomHolder.kt +++ b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt @@ -21,7 +21,7 @@ import javax.inject.Inject * This works as a FILO (First In Last Out) stack, meaning that the last room added for a session will be the first one to be removed. */ @SingleIn(AppScope::class) -class ActiveRoomHolder @Inject constructor() { +class ActiveRoomsHolder @Inject constructor() { private val rooms = ConcurrentHashMap>() /** From 9fc27298425aa1f9b78ae2d1024f53893a253fa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 23 May 2025 16:02:21 +0200 Subject: [PATCH 7/9] Add `ActiveRoomsHolder.clear(sessionId)` to clear all rooms associated with a session before clearing the cache --- .../preferences/impl/tasks/ClearCacheUseCase.kt | 3 +-- .../services/appnavstate/api/ActiveRoomsHolder.kt | 11 +++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt index 494c25bff35..5662e4fd46c 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt @@ -42,8 +42,7 @@ class DefaultClearCacheUseCase @Inject constructor( ) : ClearCacheUseCase { override suspend fun invoke() = withContext(coroutineDispatchers.io) { // Active rooms should be disposed of before clearing the cache - activeRoomHolder.removeRoom(matrixClient.sessionId)?.destroy() - activeRoomsHolder.removeRoom(matrixClient.sessionId) + activeRoomsHolder.clear(matrixClient.sessionId) // Clear Matrix cache matrixClient.clearCache() // Clear Coil cache diff --git a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt index 613c2191484..5252afcc36e 100644 --- a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt +++ b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt @@ -56,4 +56,15 @@ class ActiveRoomsHolder @Inject constructor() { val roomsForSessionId = rooms[sessionId] ?: return null return roomsForSessionId.removeLastOrNull() } + + /** + * Clears all the rooms for the given sessionId. + */ + fun clear(sessionId: SessionId) { + val activeRooms = rooms.remove(sessionId) ?: return + for (room in activeRooms) { + // Destroy the room to reset the live timelines + room.destroy() + } + } } From beb2952318ffd1dbc31a8285b7206ce80ce63e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 23 May 2025 16:03:29 +0200 Subject: [PATCH 8/9] Add `roomId` param to `ActiveRoomsHolder.removeRoom` so we can decide which room to remove --- .../appnav/room/joined/JoinedRoomLoadedFlowNode.kt | 3 +-- .../services/appnavstate/api/ActiveRoomsHolder.kt | 10 ++++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt index ef308eac3aa..c6b42e1d117 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt @@ -98,8 +98,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor( }, onDestroy = { Timber.v("OnDestroy") - activeRoomHolder.removeRoom(inputs.room.sessionId) - activeRoomsHolder.removeRoom(inputs.room.sessionId) + activeRoomsHolder.removeRoom(inputs.room.sessionId, inputs.room.roomId) inputs.room.destroy() appNavigationStateService.onLeavingRoom(id) } diff --git a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt index 5252afcc36e..2459476fcdd 100644 --- a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt +++ b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt @@ -50,11 +50,13 @@ class ActiveRoomsHolder @Inject constructor() { } /** - * Removes the last room added for the given [sessionId] and returns it or null if there weren't any. + * Removes any room matching the provided [sessionId] and [roomId]. + * + * @return true if a room was removed, false otherwise. */ - fun removeRoom(sessionId: SessionId): JoinedRoom? { - val roomsForSessionId = rooms[sessionId] ?: return null - return roomsForSessionId.removeLastOrNull() + fun removeRoom(sessionId: SessionId, roomId: RoomId): Boolean { + val roomsForSessionId = rooms[sessionId] ?: return false + return roomsForSessionId.removeIf { it.roomId == roomId } } /** From ec2e4482ef67075fd84517743054125c5fd6f07d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 23 May 2025 16:04:23 +0200 Subject: [PATCH 9/9] Change the list of active rooms to a set so you can't have the same instance twice, remove outdated docs --- .../android/services/appnavstate/api/ActiveRoomsHolder.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt index 2459476fcdd..6a9cb9f0583 100644 --- a/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt +++ b/services/appnavstate/api/src/main/kotlin/io/element/android/services/appnavstate/api/ActiveRoomsHolder.kt @@ -17,18 +17,16 @@ import javax.inject.Inject /** * Holds the active rooms for a given session so they can be reused instead of instantiating new ones. - * - * This works as a FILO (First In Last Out) stack, meaning that the last room added for a session will be the first one to be removed. */ @SingleIn(AppScope::class) class ActiveRoomsHolder @Inject constructor() { - private val rooms = ConcurrentHashMap>() + private val rooms = ConcurrentHashMap>() /** * Adds a new held room for the given sessionId. */ fun addRoom(room: JoinedRoom) { - val roomsForSessionId = rooms.getOrPut(key = room.sessionId, defaultValue = { mutableListOf() }) + val roomsForSessionId = rooms.getOrPut(key = room.sessionId, defaultValue = { mutableSetOf() }) if (roomsForSessionId.none { it.roomId == room.roomId }) { // We don't want to add the same room multiple times roomsForSessionId.add(room)