Skip to content

Add ActiveRoomHolder to manage the active room for a session #4758

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.

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 9 commits into
base: develop
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -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.ActiveRoomsHolder
import io.element.android.services.appnavstate.api.AppNavigationStateService
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
Expand All @@ -51,6 +52,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor(
private val appNavigationStateService: AppNavigationStateService,
private val appCoroutineScope: CoroutineScope,
private val matrixClient: MatrixClient,
private val activeRoomsHolder: ActiveRoomsHolder,
roomComponentFactory: RoomComponentFactory,
) : BaseFlowNode<JoinedRoomLoadedFlowNode.NavTarget>(
backstack = BackStack(
Expand Down Expand Up @@ -85,6 +87,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor(
onCreate = {
Timber.v("OnCreate => ${inputs.room.roomId}")
appNavigationStateService.onNavigateToRoom(id, inputs.room.roomId)
activeRoomsHolder.addRoom(inputs.room)
fetchRoomMembers()
trackVisitedRoom()
},
Expand All @@ -95,6 +98,7 @@ class JoinedRoomLoadedFlowNode @AssistedInject constructor(
},
onDestroy = {
Timber.v("OnDestroy")
activeRoomsHolder.removeRoom(inputs.room.sessionId, inputs.room.roomId)
inputs.room.destroy()
appNavigationStateService.onLeavingRoom(id)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.ActiveRoomsHolder
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()

Expand Down Expand Up @@ -100,6 +102,7 @@ class JoinBaseRoomLoadedFlowNodeTest {
plugins: List<Plugin>,
messagesEntryPoint: MessagesEntryPoint = FakeMessagesEntryPoint(),
roomDetailsEntryPoint: RoomDetailsEntryPoint = FakeRoomDetailsEntryPoint(),
activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(),
coroutineScope: CoroutineScope,
) = JoinedRoomLoadedFlowNode(
buildContext = BuildContext.root(savedStateMap = null),
Expand All @@ -110,6 +113,7 @@ class JoinBaseRoomLoadedFlowNodeTest {
appCoroutineScope = coroutineScope,
roomComponentFactory = FakeRoomComponentFactory(),
matrixClient = FakeMatrixClient(),
activeRoomsHolder = activeRoomsHolder,
)

@Test
Expand Down Expand Up @@ -154,4 +158,55 @@ class JoinBaseRoomLoadedFlowNodeTest {
val roomDetailsNode = roomFlowNode.childNode(JoinedRoomLoadedFlowNode.NavTarget.RoomDetails)!!
assertThat(roomDetailsNode.id).isEqualTo(fakeRoomDetailsEntryPoint.nodeId)
}

@Test
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 activeRoomsHolder = ActiveRoomsHolder()
val roomFlowNode = createJoinedRoomLoadedFlowNode(
plugins = listOf(inputs),
messagesEntryPoint = fakeMessagesEntryPoint,
roomDetailsEntryPoint = fakeRoomDetailsEntryPoint,
coroutineScope = this,
activeRoomsHolder = activeRoomsHolder,
)

assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNull()
val roomFlowNodeTestHelper = roomFlowNode.parentNodeTestHelper()
// WHEN
roomFlowNodeTestHelper.assertChildHasLifecycle(JoinedRoomLoadedFlowNode.NavTarget.Messages(null), Lifecycle.State.CREATED)
// THEN
assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNotNull()
}

@Test
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 activeRoomsHolder = ActiveRoomsHolder().apply {
addRoom(room)
}
val roomFlowNode = createJoinedRoomLoadedFlowNode(
plugins = listOf(inputs),
messagesEntryPoint = fakeMessagesEntryPoint,
roomDetailsEntryPoint = fakeRoomDetailsEntryPoint,
coroutineScope = this,
activeRoomsHolder = activeRoomsHolder,
)
val roomFlowNodeTestHelper = roomFlowNode.parentNodeTestHelper()
roomFlowNodeTestHelper.assertChildHasLifecycle(JoinedRoomLoadedFlowNode.NavTarget.Messages(null), Lifecycle.State.CREATED)
assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNotNull()
// WHEN
roomFlowNode.updateLifecycleState(Lifecycle.State.DESTROYED)
// THEN
roomFlowNodeTestHelper.assertChildHasLifecycle(JoinedRoomLoadedFlowNode.NavTarget.Messages(null), Lifecycle.State.DESTROYED)
assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNull()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.ActiveRoomsHolder
import io.element.android.services.appnavstate.api.AppForegroundStateService
import io.element.android.services.toolbox.api.systemclock.SystemClock
import kotlinx.coroutines.CoroutineScope
Expand All @@ -62,6 +63,7 @@ class CallScreenPresenter @AssistedInject constructor(
private val activeCallManager: ActiveCallManager,
private val languageTagProvider: LanguageTagProvider,
private val appForegroundStateService: AppForegroundStateService,
private val activeRoomsHolder: ActiveRoomsHolder,
private val appCoroutineScope: CoroutineScope,
) : Presenter<CallScreenState> {
@AssistedFactory
Expand Down Expand Up @@ -241,8 +243,10 @@ class CallScreenPresenter @AssistedInject constructor(

private suspend fun MatrixClient.notifyCallStartIfNeeded(roomId: RoomId) {
if (!notifiedCallStart) {
getJoinedRoom(roomId)?.use { it.sendCallNotificationIfNeeded() }
?.onSuccess { notifiedCallStart = true }
val activeRoomForSession = activeRoomsHolder.getActiveRoomMatching(sessionId, roomId)
val sendCallNotificationResult = activeRoomForSession?.sendCallNotificationIfNeeded()
?: getJoinedRoom(roomId)?.use { it.sendCallNotificationIfNeeded() }
sendCallNotificationResult?.onSuccess { notifiedCallStart = true }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.ActiveRoomsHolder
import kotlinx.coroutines.flow.firstOrNull
import javax.inject.Inject

Expand All @@ -24,6 +25,7 @@ class DefaultCallWidgetProvider @Inject constructor(
private val matrixClientsProvider: MatrixClientProvider,
private val appPreferencesStore: AppPreferencesStore,
private val callWidgetSettingsProvider: CallWidgetSettingsProvider,
private val activeRoomsHolder: ActiveRoomsHolder,
) : CallWidgetProvider {
override suspend fun getWidget(
sessionId: SessionId,
Expand All @@ -33,7 +35,9 @@ class DefaultCallWidgetProvider @Inject constructor(
theme: String?,
): Result<CallWidgetProvider.GetWidgetResult> = runCatching {
val matrixClient = matrixClientsProvider.getOrRestore(sessionId).getOrThrow()
val room = matrixClient.getJoinedRoom(roomId) ?: error("Room not found")
val room = activeRoomsHolder.getActiveRoomMatching(sessionId, roomId)
?: matrixClient.getJoinedRoom(roomId)
?: error("Room not found")

val customBaseUrl = appPreferencesStore.getCustomElementCallBaseUrlFlow().firstOrNull()
val baseUrl = customBaseUrl ?: EMBEDDED_CALL_WIDGET_BASE_URL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.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
Expand Down Expand Up @@ -367,6 +368,7 @@ import kotlin.time.Duration.Companion.seconds
activeCallManager: FakeActiveCallManager = FakeActiveCallManager(),
screenTracker: ScreenTracker = FakeScreenTracker(),
appForegroundStateService: FakeAppForegroundStateService = FakeAppForegroundStateService(),
activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(),
): CallScreenPresenter {
val userAgentProvider = object : UserAgentProvider {
override fun provide(): String {
Expand All @@ -387,6 +389,7 @@ import kotlin.time.Duration.Companion.seconds
languageTagProvider = FakeLanguageTagProvider("en-US"),
appForegroundStateService = appForegroundStateService,
appCoroutineScope = backgroundScope,
activeRoomsHolder = activeRoomsHolder,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.ActiveRoomsHolder
import kotlinx.coroutines.test.runTest
import org.junit.Test

Expand Down Expand Up @@ -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 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) },
activeRoomsHolder = activeRoomsHolder
)
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(
Expand Down Expand Up @@ -104,9 +123,11 @@ class DefaultCallWidgetProviderTest {
matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider(),
appPreferencesStore: AppPreferencesStore = InMemoryAppPreferencesStore(),
callWidgetSettingsProvider: CallWidgetSettingsProvider = FakeCallWidgetSettingsProvider(),
activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(),
) = DefaultCallWidgetProvider(
matrixClientsProvider = matrixClientProvider,
appPreferencesStore = appPreferencesStore,
callWidgetSettingsProvider = callWidgetSettingsProvider,
activeRoomsHolder = activeRoomsHolder,
)
}
1 change: 1 addition & 0 deletions features/preferences/impl/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.ActiveRoomsHolder
import kotlinx.coroutines.withContext
import okhttp3.OkHttpClient
import javax.inject.Inject
Expand All @@ -37,8 +38,11 @@ class DefaultClearCacheUseCase @Inject constructor(
private val ftueService: FtueService,
private val pushService: PushService,
private val seenInvitesStore: SeenInvitesStore,
private val activeRoomsHolder: ActiveRoomsHolder,
) : ClearCacheUseCase {
override suspend fun invoke() = withContext(coroutineDispatchers.io) {
// Active rooms should be disposed of before clearing the cache
activeRoomsHolder.clear(matrixClient.sessionId)
// Clear Matrix cache
matrixClient.clearCache()
// Clear Coil cache
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.ActiveRoomsHolder
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value
import io.element.android.tests.testutils.testCoroutineDispatchers
Expand All @@ -31,8 +34,10 @@ import org.robolectric.RobolectricTestRunner
class DefaultClearCacheUseCaseTest {
@Test
fun `execute clear cache should do all the expected tasks`() = runTest {
val activeRoomsHolder = ActiveRoomsHolder().apply { addRoom(FakeJoinedRoom()) }
val clearCacheLambda = lambdaRecorder<Unit> { }
val matrixClient = FakeMatrixClient(
sessionId = A_SESSION_ID,
clearCacheLambda = clearCacheLambda,
)
val defaultCacheService = DefaultCacheService()
Expand All @@ -55,6 +60,7 @@ class DefaultClearCacheUseCaseTest {
ftueService = ftueService,
pushService = pushService,
seenInvitesStore = seenInvitesStore,
activeRoomsHolder = activeRoomsHolder,
)
defaultCacheService.clearedCacheEventFlow.test {
sut.invoke()
Expand All @@ -64,6 +70,7 @@ class DefaultClearCacheUseCaseTest {
.with(value(matrixClient.sessionId), value(false))
assertThat(awaitItem()).isEqualTo(matrixClient.sessionId)
assertThat(seenInvitesStore.seenRoomIds().first()).isEmpty()
assertThat(activeRoomsHolder.getActiveRoom(A_SESSION_ID)).isNull()
}
}
}
1 change: 1 addition & 0 deletions features/share/impl/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading